Key S3 client cache by config value, not pointer

What does this MR do?

Fixes the unbounded growth of the s3v2 client cache, which is at least a large part of the runner-manager heap leak tracked in #39595 (closed).

Root cause

The package-level s3ClientCache in cache/s3v2/s3.go is a sync.Map of lazily-built AWS S3 clients. It was keyed by *cacheconfig.CacheS3Config pointer identity. The comment on the old code assumed pointers were stable per config load, so pointer identity would capture "which runner, which load."

That assumption doesn't survive contact with common.NewBuild, which deep-copies RunnerConfig through JSON for every job. Every cache-using job presents a config pointer the map has never seen, so every job builds a brand-new AWS client and stores it forever. Measured retention is about 11 KB per cache-using job, which lines up with the 2.1 to 4.5 KB/job fleet-wide slopes we measured on GitLab Dedicated hosted runners (job mix there is a blend of cache-using and cache-free jobs). The production path is confirmed: Dedicated enables the S3 cache unconditionally, GRIT renders Type = "s3", and FF_USE_LEGACY_S3_CACHE_ADAPTER defaults to false, which selects this adapter.

The fix

Key the cache by the effective config value instead of the pointer. newS3ClientCacheKey builds a comparable struct from the config, flattening the two pointer-valued options (DualStack, PathStyle) into explicit set/value pairs so JSON deep copies of the same config compare equal. Any real configuration change still produces a distinct key and therefore a fresh client.

A few things came out of review and are part of this MR:

• Configs without an explicit BucketLocation now always bypass the cache. Region auto-detection falls back to us-east-1 on transient errors, and caching that result would pin a wrong-region client for the life of the process. Bypassing keeps the old self-healing behavior.
• buildS3Client stores a private deep copy of the config, so callers holding different but equal-valued pointers never alias one caller's mutable struct.
• newS3Client(nil) returns an error instead of panicking.
• A compile-time guard (var _ = map[s3ClientCacheKey]struct{}{}) turns a future non-comparable field in CacheS3Config into a build failure rather than a runtime panic in LoadOrStore.
• A nil DualStack/PathStyle and an explicitly set one are deliberately distinct keys, even when they resolve to the same enabled state. Collapsing them would risk sharing a client across configs that PathStyleEnabled() auto-detects differently; keeping them separate costs one extra cached client at most.

Tests

The main regression test deep-copies the config through JSON 1,000 times, exactly the way RunnerConfig.DeepCopy does, and asserts all copies share one client. I verified it fails on every iteration without the fix. New tests also cover nil-pointer normalization, the nil-vs-set key distinction, concurrent first access (runs under -race), the build-error eviction and retry path, the no-aliasing invariant, and the empty-BucketLocation bypass. Pre-existing tests that populate the now value-keyed global cache got cleanup added so they stay isolated.

go build, go vet, go test -race ./cache/s3v2 -count=1, and make lint all pass.

Known limits

The cache is still unbounded across distinct effective configs: one entry per config value, including one stale entry per changed config after a reload. Config values are operator-controlled, so cardinality is small in practice. A flush-on-reload hook (like FlushCredentialCache but for this map) would clean these up; that's tracked in #39600 rather than growing this MR.

One behavior note: a config reload that leaves S3 values unchanged now reuses the cached client, so ambient process state read at construction time (the AWS_RESPONSE_CHECKSUM_VALIDATION/AWS_REQUEST_CHECKSUM_CALCULATION env vars, AWS_PROFILE source selection) needs a process restart to take effect. Credential rotation keeps working for expiring providers (IMDS, ECS, AssumeRole, SSO, IRSA), because the SDK's provider chain refreshes those internally. Static credentials read from a shared credentials file via AWS_PROFILE are the exception: the SDK reads them once at client construction and never re-reads the file, so rotating that file now requires a process restart (previously each job rebuilt the client and picked up the new file contents). AccessKey/SecretKey set in config.toml are unaffected since they're part of the cache key. The file-based case is also covered by the flush-on-reload follow-up in #39600. This is documented in the code.

Related issues

Closes #39595 (closed)

Follow-up: #39600

Author's checklist

• Follow the Style Guide
• Documentation added/updated (code comments; no user-facing doc change needed)
• Added tests for this feature/bug
• Set the milestone