amadan

carlos / platform Public

Revert "lease: one S3 client per region and inputs, not one per wake"

This reverts commit 4e85d529e44b3adfb0773bb88eb4f90dc69c2ce8.
claude a911e5360a07f2aa897180f16b71e3d73b1cc2c7 parent 16bd14d
3 files changed, +0 −190
  • internal/lease/clientcache.go +0 −66
  • internal/lease/clientcache_test.go +0 −106
  • internal/lease/endpoint.go +0 −18
diff --git a/internal/lease/clientcache.go b/internal/lease/clientcache.go
deleted file mode 100644
index d5f2d4601..000000000
--- a/internal/lease/clientcache.go
+++ /dev/null
@@ -1,66 +0,0 @@
-package lease
-
-import (
- "crypto/sha256"
- "encoding/hex"
- "fmt"
- "os"
- "path/filepath"
- "sort"
- "strings"
- "sync"
-
- awss3 "github.com/aws/aws-sdk-go-v2/service/s3"
-)
-
-// Every wake takes a lease, and building the lease's S3 client was a fresh
-// AWS config load and a fresh credentials lookup each time: part of the
-// 130 ms the lease step took on a dev wake (2026-10-24). One client per set
-// of inputs is kept instead. An S3 client is safe for concurrent use, and
-// credentials that expire (an instance profile's) are refreshed by the
-// SDK's own cache inside it, so sharing changes nothing a fresh load would
-// decide. Anything a fresh load reads is part of the key, so a change to
-// it builds a new client: the region asked for, the environment the SDK
-// and s3compat read (AWS_*, CARLOS_S3_*), the effective uid and home
-// directory, and the identity of every credentials or config file the
-// load could open (rootaws's /etc/carlos/credentials for root, the shared
-// files otherwise).
-type leaserClient struct {
- client *awss3.Client
- region string
-}
-
-var leaserClients sync.Map // key string -> leaserClient
-
-func resetLeaserClients() { leaserClients.Clear() }
-
-func leaserClientKey(region string) string {
- var env []string
- for _, kv := range os.Environ() {
- if strings.HasPrefix(kv, "AWS_") || strings.HasPrefix(kv, "CARLOS_S3_") {
- env = append(env, kv)
- }
- }
- sort.Strings(env)
- home, _ := os.UserHomeDir()
- files := []string{"/etc/carlos/credentials", os.Getenv("AWS_SHARED_CREDENTIALS_FILE"), os.Getenv("AWS_CONFIG_FILE")}
- if home != "" {
- files = append(files, filepath.Join(home, ".aws", "credentials"), filepath.Join(home, ".aws", "config"))
- }
- h := sha256.New()
- fmt.Fprintf(h, "region=%s\x00uid=%d\x00home=%s\x00", region, os.Geteuid(), home)
- for _, kv := range env {
- fmt.Fprintf(h, "env=%s\x00", kv)
- }
- for _, f := range files {
- if f == "" {
- continue
- }
- if st, err := os.Stat(f); err == nil {
- fmt.Fprintf(h, "file=%s:%d:%d:%v\x00", f, st.Size(), st.ModTime().UnixNano(), st.Mode())
- } else {
- fmt.Fprintf(h, "file=%s:absent\x00", f)
- }
- }
- return hex.EncodeToString(h.Sum(nil))
-}
diff --git a/internal/lease/clientcache_test.go b/internal/lease/clientcache_test.go
deleted file mode 100644
index 2cc6725db..000000000
--- a/internal/lease/clientcache_test.go
+++ /dev/null
@@ -1,106 +0,0 @@
-package lease
-
-import (
- "context"
- "os"
- "path/filepath"
- "testing"
- "time"
-)
-
-func cacheTestEnv(t *testing.T) {
- t.Helper()
- t.Setenv("AWS_ACCESS_KEY_ID", "test-key")
- t.Setenv("AWS_SECRET_ACCESS_KEY", "test-secret")
- t.Setenv("AWS_SESSION_TOKEN", "")
- t.Setenv("AWS_EC2_METADATA_DISABLED", "true")
- t.Setenv("CARLOS_S3_ENDPOINT", "")
- resetLeaserClients()
- t.Cleanup(resetLeaserClients)
-}
-
-// A wake builds its lease client once per region and endpoint, not once per
-// wake: the config load and the credentials lookup are paid by the first.
-func TestLeaserClientIsReusedForTheSameRegion(t *testing.T) {
- cacheTestEnv(t)
- a, ra, err := newLeaserClient(context.Background(), "eu-west-1")
- if err != nil {
- t.Fatal(err)
- }
- b, rb, err := newLeaserClient(context.Background(), "eu-west-1")
- if err != nil {
- t.Fatal(err)
- }
- if a != b || ra != rb {
- t.Fatalf("second call built a new client: %p %p (%q %q)", a, b, ra, rb)
- }
- c, _, err := newLeaserClient(context.Background(), "us-east-1")
- if err != nil {
- t.Fatal(err)
- }
- if c == a {
- t.Fatal("another region shared the client")
- }
-}
-
-// Anything that changes what a fresh load would build gives a fresh client:
-// the endpoint override, the environment's keys, a shared credentials file.
-func TestLeaserClientIsRebuiltWhenItsInputsChange(t *testing.T) {
- cacheTestEnv(t)
- first, _, err := newLeaserClient(context.Background(), "eu-west-1")
- if err != nil {
- t.Fatal(err)
- }
- for name, change := range map[string]func(t *testing.T){
- "the endpoint": func(t *testing.T) { t.Setenv("CARLOS_S3_ENDPOINT", "https://fsn1.your-objectstorage.com") },
- "the keys": func(t *testing.T) { t.Setenv("AWS_ACCESS_KEY_ID", "other-key") },
- "the s3 quirks": func(t *testing.T) { t.Setenv("CARLOS_S3_QUIRKS", "unquoted-if-match") },
- "the env region": func(t *testing.T) { t.Setenv("AWS_DEFAULT_REGION", "eu-central-1") },
- "the creds file": func(t *testing.T) {
- path := filepath.Join(t.TempDir(), "credentials")
- if err := os.WriteFile(path, []byte("[default]\n"), 0o600); err != nil {
- t.Fatal(err)
- }
- t.Setenv("AWS_SHARED_CREDENTIALS_FILE", path)
- },
- } {
- t.Run(name, func(t *testing.T) {
- change(t)
- c, _, err := newLeaserClient(context.Background(), "eu-west-1")
- if err != nil {
- t.Fatal(err)
- }
- if c == first {
- t.Fatalf("%s changed but the client was reused", name)
- }
- })
- }
-}
-
-// A credentials file rewritten in place (same path) is a new input too.
-func TestLeaserClientIsRebuiltWhenTheCredentialsFileChanges(t *testing.T) {
- cacheTestEnv(t)
- path := filepath.Join(t.TempDir(), "credentials")
- if err := os.WriteFile(path, []byte("[default]\n"), 0o600); err != nil {
- t.Fatal(err)
- }
- t.Setenv("AWS_SHARED_CREDENTIALS_FILE", path)
- first, _, err := newLeaserClient(context.Background(), "eu-west-1")
- if err != nil {
- t.Fatal(err)
- }
- later := time.Now().Add(time.Minute)
- if err := os.WriteFile(path, []byte("[default]\nregion = eu-west-1\n"), 0o600); err != nil {
- t.Fatal(err)
- }
- if err := os.Chtimes(path, later, later); err != nil {
- t.Fatal(err)
- }
- c, _, err := newLeaserClient(context.Background(), "eu-west-1")
- if err != nil {
- t.Fatal(err)
- }
- if c == first {
- t.Fatal("the credentials file changed but the client was reused")
- }
-}
diff --git a/internal/lease/endpoint.go b/internal/lease/endpoint.go
index 58047f041..4ec0156a9 100644
--- a/internal/lease/endpoint.go
+++ b/internal/lease/endpoint.go
@@ -101,24 +101,6 @@ func regionFor(explicit, chainResolved string) string {
// carry that exact value onto the Held for Replicate's replica client rather
// than letting litestream substitute a different default of its own.
func newLeaserClient(ctx context.Context, region string) (*awss3.Client, string, error) {
- // One client per set of inputs (clientcache.go): the config load and
- // credentials lookup are a wake's, once, not every wake's.
- key := leaserClientKey(region)
- if c, ok := leaserClients.Load(key); ok {
- lc := c.(leaserClient)
- return lc.client, lc.region, nil
- }
- client, resolved, err := buildLeaserClient(ctx, region)
- if err != nil {
- return nil, "", err
- }
- c, _ := leaserClients.LoadOrStore(key, leaserClient{client: client, region: resolved})
- lc := c.(leaserClient)
- return lc.client, lc.region, nil
-}
-
-// buildLeaserClient is newLeaserClient's uncached body.
-func buildLeaserClient(ctx context.Context, region string) (*awss3.Client, string, error) {
var opts []func(*awsconfig.LoadOptions) error
if region != "" {
opts = append(opts, awsconfig.WithRegion(region))