Skip to content

secret: the secreter is never closed, so every hot reload leaks its cache and goroutines #1000

Description

@Michad

The secreter is built on every configuration load but never closed, so each hot reload leaks its cache and the background goroutines that maintain it.

Entities holds every entity that needs releasing, pkg/entities/entities.go:35-41:

type Entities struct {
	LayerGroup *layer.LayerGroup
	Auth       authentication.Authentication
	Analytics  *analytics.AnalyticsWrapper
	Caches     *cache.CacheRegistry
	Datastores *datastore.DatastoreRegistry
}

There's no secreter field, so Entities.Close has nothing to call. In pkg/entry/utility.go:41 the secreter is a local, passed into the datastores, caches, auth, analytics and layers and then dropped. The Secreter interface (pkg/entities/secret/secreter.go:25-27) has no close hook to call either.

The AWS implementation owns real resources, internal/secrets/aws_secrets_manager.go:107-116:

if cfg.TTL > 0 {
	cache, err := otter.MustBuilder[string, string](cacheSize).WithTTL(time.Duration(cfg.TTL) * time.Second).Build()
	...
	return &AWSSecretsManager{cfg, svc, &cache}, nil
}

cacheSize is 10,000. Otter starts two goroutines when built this way: cleanup(), which loops on a one second sleep until isClosed, and process(), which blocks on the write buffer until a close task arrives. Only Cache.Close() sets either, and nothing ever calls it. The unixtime.Start() refcount otter takes is never balanced by Stop() either.

configToEntities runs once at startup (pkg/entry/serve.go:36) and again on every reload (serve.go:64), so with secret: {name: awssecretsmanager, ttl: ...} and --hot-reload, each reload adds another 10,000-entry cache and two more goroutines that never exit. Unbounded, and it accumulates fastest in exactly the long-running deployments that reload most.

This is the same class as the cache and datastore close paths that already exist; secrets just never got wired in. docs/operation/modules/ROOT/pages/reloading.adoc tells operators the opposite is true:

When reloading occurs, new connections will be established for configured datastores and caches. The previous connections are closed once in-flight requests using them have had time to complete, therefore the number of connections returns to its steady state instead of growing with each reload.

and lists secret as a reloadable section.

Unlike most of these this isn't a one-liner. It needs an optional close hook on Secreter (matching how providers are handled, where implementing Closer is optional), a field on Entities, and a call in Entities.Close ordered so the secreter outlives anything that might still resolve a secret during shutdown. A Close on AWSSecretsManager that closes the otter cache, and a check of whether the other secret implementations hold anything similar, come with it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions