diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 39a64ff..0c322cd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -66,8 +66,8 @@ and `docker.io/blockossreleases/spectre`. `spectre serve` collects the quarantines that mirrors report. It listens on `127.0.0.1:50070` by default and keeps them in memory, so a restart loses them. -To keep them in Redis instead, pass `--storage=redis` and `--redis-url`, such as -`redis://localhost:6379/0`. +To keep them in Redis instead, pass a Redis URL to `--storage`, such as +`--storage=redis://localhost:6379/0`. Every mirror reports to a server, so `spectre supervise` requires its URL, `--server-url`, and the mirror's identity, `--service`, `--environment`, and diff --git a/cli_internal_test.go b/cli_internal_test.go index d1924dd..227be64 100644 --- a/cli_internal_test.go +++ b/cli_internal_test.go @@ -2,6 +2,7 @@ package spectre import ( "fmt" + "net/url" "os" "path/filepath" "slices" @@ -124,11 +125,11 @@ func TestServeCommandEmbedsConfig(t *testing.T) { command := &cli.CommandLine{} parser, err := newParser(command) assert.NoError(t, err) - _, err = parser.Parse([]string{"serve", "--listen=127.0.0.1:50071", "--storage=redis", "--redis-url=redis://localhost:6379/0"}) + _, err = parser.Parse([]string{"serve", "--listen=127.0.0.1:50071", "--storage=redis://localhost:6379/0"}) assert.NoError(t, err) expected := server.ServeConfig{ Listen: "127.0.0.1:50071", - Storage: storage.Config{Storage: storage.BackendRedis, RedisURL: "redis://localhost:6379/0"}, + Storage: storage.Config{Storage: &url.URL{Scheme: "redis", Host: "localhost:6379", Path: "/0"}}, } assert.Equal(t, expected, command.Serve.Serve) } diff --git a/internal/server/storage/memory.go b/internal/server/storage/memory.go index 45e0ed3..15ef748 100644 --- a/internal/server/storage/memory.go +++ b/internal/server/storage/memory.go @@ -2,11 +2,16 @@ package storage import ( "context" + "net/url" "sync" "github.com/block/spectre/internal/model" ) +func init() { + register("memory", func(context.Context, *url.URL) (Store, error) { return NewMemory(), nil }) +} + // Memory keeps divergences in memory, so a restart loses them. Stored // divergences share their slices with callers, who never mutate them. type Memory struct { diff --git a/internal/server/storage/redis.go b/internal/server/storage/redis.go index cfd11d0..32c5146 100644 --- a/internal/server/storage/redis.go +++ b/internal/server/storage/redis.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "net/url" "strconv" "github.com/alecthomas/errors" @@ -12,6 +13,19 @@ import ( "github.com/block/spectre/internal/model" ) +func init() { + for _, scheme := range []string{"redis", "rediss"} { + register(scheme, func(ctx context.Context, storageURL *url.URL) (Store, error) { + // Returning a nil *Redis directly would make a non-nil Store. + store, err := NewRedis(ctx, storageURL) + if err != nil { + return nil, err + } + return store, nil + }) + } +} + // divergencesKey is the Redis hash that maps each sourceField to its divergence // as JSON. const divergencesKey = "spectre:divergences" @@ -24,9 +38,9 @@ type Redis struct { var _ Store = (*Redis)(nil) -// NewRedis connects to the Redis server the configuration names. -func NewRedis(ctx context.Context, config Config) (*Redis, error) { - options, err := redis.ParseURL(config.RedisURL) +// NewRedis connects to the Redis server at a redis:// or rediss:// URL. +func NewRedis(ctx context.Context, storageURL *url.URL) (*Redis, error) { + options, err := redis.ParseURL(storageURL.String()) if err != nil { return nil, errors.Wrap(err, "redis URL") } diff --git a/internal/server/storage/storage.go b/internal/server/storage/storage.go index 94777ae..b19337a 100644 --- a/internal/server/storage/storage.go +++ b/internal/server/storage/storage.go @@ -1,46 +1,48 @@ // Package storage keeps the divergences that mirrors report to the SPECTRE -// server, either in memory or in Redis. +// server. A URL selects the backend by its scheme, such as memory:// or redis://. package storage import ( "context" + "net/url" "github.com/alecthomas/errors" - "github.com/redis/go-redis/v9" "github.com/block/spectre/internal/model" ) -// Backend names where the SPECTRE server keeps divergences. -type Backend string +// Constructor opens a store at a URL whose scheme it is registered for. +type Constructor func(ctx context.Context, storageURL *url.URL) (Store, error) -// Backends the SPECTRE server can keep divergences in. -const ( - BackendMemory Backend = "memory" - BackendRedis Backend = "redis" -) +// constructors maps each URL scheme to its backend. Backends write it only from +// init, so reading it needs no lock. +var constructors = map[string]Constructor{} //nolint:gochecknoglobals // Backends register themselves. + +// register makes Open use constructor for URLs with scheme. Call it only from init. +func register(scheme string, constructor Constructor) { + constructors[scheme] = constructor +} // Config selects where the SPECTRE server keeps divergences. type Config struct { - Storage Backend `default:"memory" enum:"memory,redis" help:"Where to keep divergences: memory, which a restart loses, or redis."` - RedisURL string `name:"redis-url" placeholder:"URL" help:"URL of the Redis server that keeps divergences when storage is redis, such as redis://localhost:6379/0."` + Storage *url.URL `default:"memory://" placeholder:"URL" help:"Where to keep divergences: memory://, which a restart loses, or a Redis URL such as redis://localhost:6379/0."` } -// Validate checks that the selected backend is configured. +// Validate checks that a backend serves the storage URL's scheme. func (c Config) Validate() error { - switch c.Storage { - case BackendMemory: - return nil - case BackendRedis: - if c.RedisURL == "" { - return errors.New("redis storage needs a Redis URL") - } - if _, err := redis.ParseURL(c.RedisURL); err != nil { - return errors.Wrap(err, "redis URL") - } - return nil + _, err := c.constructor() + return err +} + +func (c Config) constructor() (Constructor, error) { + if c.Storage == nil { + return nil, errors.New("storage URL is required") + } + constructor, ok := constructors[c.Storage.Scheme] + if !ok { + return nil, errors.Errorf("unknown storage scheme %q", c.Storage.Scheme) } - return errors.Errorf("unknown storage %q", c.Storage) + return constructor, nil } // Store keeps the latest divergence of each mirror direction. Callers validate @@ -55,18 +57,13 @@ type Store interface { Close() error } -// Open returns the store the configuration selects. +// Open returns the store the configuration's URL selects. func Open(ctx context.Context, config Config) (Store, error) { - if err := config.Validate(); err != nil { + constructor, err := config.constructor() + if err != nil { return nil, err } - switch config.Storage { - case BackendMemory: - return NewMemory(), nil - case BackendRedis: - return NewRedis(ctx, config) - } - return nil, errors.Errorf("unknown storage %q", config.Storage) + return constructor(ctx, config.Storage) } // quarantinedAfter reports whether existing began after incoming. A retry from a diff --git a/internal/server/storage/storage_test.go b/internal/server/storage/storage_test.go index 0751600..90a9a94 100644 --- a/internal/server/storage/storage_test.go +++ b/internal/server/storage/storage_test.go @@ -2,6 +2,7 @@ package storage_test import ( "cmp" + "net/url" "slices" "strconv" "sync" @@ -48,20 +49,19 @@ func TestStores(t *testing.T) { func TestOpen(t *testing.T) { server := miniredis.RunT(t) tests := []struct { - name string - config storage.Config - err string + name string + url string + err string }{ - {name: "Memory", config: storage.Config{Storage: storage.BackendMemory}}, - {name: "Redis", config: storage.Config{Storage: storage.BackendRedis, RedisURL: "redis://" + server.Addr()}}, - {name: "RedisWithoutURL", config: storage.Config{Storage: storage.BackendRedis}, err: "redis storage needs a Redis URL"}, - {name: "InvalidRedisURL", config: storage.Config{Storage: storage.BackendRedis, RedisURL: "http://localhost"}, err: "redis URL"}, - {name: "UnreachableRedis", config: storage.Config{Storage: storage.BackendRedis, RedisURL: "redis://127.0.0.1:1"}, err: "connect to Redis"}, - {name: "UnknownBackend", config: storage.Config{Storage: "disk"}, err: `unknown storage "disk"`}, + {name: "Memory", url: "memory://"}, + {name: "Redis", url: "redis://" + server.Addr()}, + {name: "InvalidRedisURL", url: "redis://localhost/notadatabase", err: "redis URL"}, + {name: "UnreachableRedis", url: "redis://127.0.0.1:1", err: "connect to Redis"}, + {name: "UnknownScheme", url: "disk:///var/lib/spectre", err: `unknown storage scheme "disk"`}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - store, err := storage.Open(t.Context(), test.config) + store, err := storage.Open(t.Context(), storage.Config{Storage: parseURL(t, test.url)}) if test.err != "" { assert.Contains(t, err.Error(), test.err) return @@ -72,9 +72,14 @@ func TestOpen(t *testing.T) { } } +func TestOpenRequiresURL(t *testing.T) { + _, err := storage.Open(t.Context(), storage.Config{}) + assert.EqualError(t, err, "storage URL is required") +} + func TestRedisRejectsUnknownFields(t *testing.T) { server := miniredis.RunT(t) - store, err := storage.NewRedis(t.Context(), storage.Config{Storage: storage.BackendRedis, RedisURL: "redis://" + server.Addr()}) + store, err := storage.NewRedis(t.Context(), parseURL(t, "redis://"+server.Addr())) assert.NoError(t, err) t.Cleanup(func() { assert.NoError(t, store.Close()) }) server.HSet("spectre:divergences", `"users" "production" ingress`, @@ -86,11 +91,18 @@ func TestRedisRejectsUnknownFields(t *testing.T) { func openRedis(t *testing.T) storage.Store { t.Helper() server := miniredis.RunT(t) - store, err := storage.NewRedis(t.Context(), storage.Config{Storage: storage.BackendRedis, RedisURL: "redis://" + server.Addr()}) + store, err := storage.NewRedis(t.Context(), parseURL(t, "redis://"+server.Addr())) assert.NoError(t, err) return store } +func parseURL(t *testing.T, raw string) *url.URL { + t.Helper() + parsed, err := url.Parse(raw) + assert.NoError(t, err) + return parsed +} + func testKeepsLatestQuarantine(t *testing.T, store storage.Store) { first := time.Date(2026, 10, 1, 12, 0, 0, 0, time.UTC) latest := divergence("production", model.Ingress, first.Add(time.Minute))