Skip to content

Commit 87482ed

Browse files
fix: set library-friendly defaults on server Config (#3156)
## Changes * Add `default:"..."` tags for scalar `Config` fields whose CLI flag default in `pkg/cmd/serve.go` is non-zero. Closes #2502 --------- Co-authored-by: Maria Ines Parnisari <maria.ines.parnisari@authzed.com>
1 parent a8858d8 commit 87482ed

13 files changed

Lines changed: 294 additions & 65 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
2121
- Tracing: When server is shutting down, flush traces. Also, elide the need for setting `OTEL_EXPORTER_OTLP_ENDPOINT`. (https://github.com/authzed/spicedb/pull/3108)
2222
- Fixed a LookupSubjects issue in the query planner around the handling of wildcards in compound permissions (https://github.com/authzed/spicedb/pull/3140)
2323
- MySQL: identifiers (object/subject IDs and relationship counter names) are now stored with a case-sensitive (binary) collation, matching the Postgres, CockroachDB, and Spanner datastores. Previously, identifiers differing only in letter case (e.g. `Foo` and `foo`) incorrectly collided in unique indexes and lookups. ⚠️ The migration rebuilds the `relation_tuple` table in place via `ALTER TABLE`, which can hold a metadata/table lock for a long time on large datasets — run the upgrade in a low-traffic window, or apply it with an online schema-change tool (e.g. gh-ost). (https://github.com/authzed/spicedb/pull/3161)
24+
- `server.NewConfigWithOptionsAndDefaults` now populates `Config` and its embedded structs with the same defaults as the CLI flags, fixing zero-value behavior when embedding SpiceDB as a library. (https://github.com/authzed/spicedb/pull/3156)
2425

2526
## [1.53.0] - 2026-05-13
2627
### Added

internal/services/integrationtesting/certtest/cert_test.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func TestCertRotation(t *testing.T) {
126126
require.NoError(t, err)
127127

128128
ctx, cancel := context.WithCancel(t.Context())
129-
srv, err := server.NewConfigWithOptionsAndDefaults(
129+
cfg := server.NewConfigWithOptionsAndDefaults(
130130
server.WithDatastore(ds),
131131
server.WithDispatcher(dispatcher),
132132
server.WithDispatchMaxDepth(50),
@@ -183,7 +183,17 @@ func TestCertRotation(t *testing.T) {
183183
},
184184
},
185185
}),
186-
).Complete(ctx)
186+
)
187+
// Disable caches and their metrics to avoid "duplicate metrics" errors
188+
cfg.DispatchClusterMetricsEnabled = false
189+
cfg.DispatchClientMetricsEnabled = false
190+
cfg.DatastoreConfig.EnableDatastoreMetrics = false
191+
cfg.NamespaceCacheConfig = server.CacheConfig{}
192+
cfg.DispatchCacheConfig = server.CacheConfig{}
193+
cfg.ClusterDispatchCacheConfig = server.CacheConfig{}
194+
cfg.LR3ResourceChunkCacheConfig = server.CacheConfig{}
195+
cfg.StoredSchemaCacheConfig = server.CacheConfig{}
196+
srv, err := cfg.Complete(ctx)
187197
require.NoError(t, err)
188198

189199
wait := make(chan error, 1)

internal/services/v1/experimental_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func TestBulkExportRelationshipsBeyondAllowedLimit(t *testing.T) {
157157

158158
_, err = resp.Recv()
159159
require.Error(err)
160-
require.Contains(err.Error(), "provided limit 10000005 is greater than maximum allowed of 100000")
160+
require.Contains(err.Error(), "provided limit 10000005 is greater than maximum allowed of 10000")
161161
}
162162

163163
func TestBulkExportRelationships(t *testing.T) {

internal/services/v1/permissions_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,9 @@ func TestCheckPermissionWithDebugInfoInError(t *testing.T) {
736736
}
737737

738738
func TestLookupResources(t *testing.T) {
739+
t.Cleanup(func() {
740+
goleak.VerifyNone(t, testutil.GoLeakIgnores()...)
741+
})
739742
testCases := []struct {
740743
objectType string
741744
permission string
@@ -920,9 +923,9 @@ func TestLookupResources(t *testing.T) {
920923
tf.StandardDatastoreWithData,
921924
)
922925
client := v1.NewPermissionsServiceClient(conn)
923-
defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
924-
defer cleanup()
925-
926+
t.Cleanup(func() {
927+
cleanup()
928+
})
926929
var trailer metadata.MD
927930
lookupClient, err := client.LookupResources(t.Context(), &v1.LookupResourcesRequest{
928931
ResourceObjectType: tc.objectType,
@@ -2469,7 +2472,7 @@ func TestExportBulkRelationshipsBeyondAllowedLimit(t *testing.T) {
24692472

24702473
_, err = resp.Recv()
24712474
require.Error(err)
2472-
require.Contains(err.Error(), "provided limit 10000005 is greater than maximum allowed of 100000")
2475+
require.Contains(err.Error(), "provided limit 10000005 is greater than maximum allowed of 10000")
24732476
}
24742477

24752478
func TestExportBulkRelationships(t *testing.T) {

internal/services/v1/relationships.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func NewPermissionsServer(
158158
MaxReadRelationshipsLimit: defaultIfZero(config.MaxReadRelationshipsLimit, 1_000),
159159
MaxDeleteRelationshipsLimit: defaultIfZero(config.MaxDeleteRelationshipsLimit, 1_000),
160160
MaxLookupResourcesLimit: defaultIfZero(config.MaxLookupResourcesLimit, 1_000),
161-
MaxBulkExportRelationshipsLimit: defaultIfZero(config.MaxBulkExportRelationshipsLimit, 100_000),
161+
MaxBulkExportRelationshipsLimit: defaultIfZero(config.MaxBulkExportRelationshipsLimit, 10_000),
162162
DispatchChunkSize: defaultIfZero(config.DispatchChunkSize, 100),
163163
MaxCheckBulkConcurrency: defaultIfZero(config.MaxCheckBulkConcurrency, 50),
164164
CaveatTypeSet: caveattypes.TypeSetOrDefault(config.CaveatTypeSet),

internal/testserver/cluster.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,15 @@ func TestClusterWithDispatch(t testing.TB, size uint, ds datastore.Datastore, ad
229229

230230
ctx, cancel := context.WithCancel(t.Context())
231231
cfg := server.NewConfigWithOptionsAndDefaults(serverOptions...)
232+
// Disable caches and their metrics to avoid "duplicate metrics" errors
233+
cfg.DispatchClusterMetricsEnabled = false
234+
cfg.DispatchClientMetricsEnabled = false
235+
cfg.DatastoreConfig.EnableDatastoreMetrics = false
236+
cfg.NamespaceCacheConfig = server.CacheConfig{}
237+
cfg.DispatchCacheConfig = server.CacheConfig{}
238+
cfg.ClusterDispatchCacheConfig = server.CacheConfig{}
239+
cfg.LR3ResourceChunkCacheConfig = server.CacheConfig{}
240+
cfg.StoredSchemaCacheConfig = server.CacheConfig{}
232241
srv, listeners, err := cfg.CompleteForTesting(ctx)
233242
require.NoError(t, err)
234243

internal/testserver/server.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func NewTestServerWithConfigAndDatastore(t testing.TB,
9595
dispatcher, err := graph.NewLocalOnlyDispatcher(params)
9696
require.NoError(t, err)
9797

98-
srv, err := server.NewConfigWithOptionsAndDefaults(
98+
cfg := server.NewConfigWithOptionsAndDefaults(
9999
server.WithDatastore(ds),
100100
server.WithDispatcher(dispatcher),
101101
server.WithQueryPlanMetadata(queryPlanMetadata),
@@ -163,7 +163,21 @@ func NewTestServerWithConfigAndDatastore(t testing.TB,
163163
},
164164
},
165165
}),
166-
).Complete(ctx)
166+
)
167+
// Disable all caching and metrics so test servers retain their pre-PR
168+
// behavior. The new library defaults enable real caches (matching CLI),
169+
// which (a) break parallel subtests by registering duplicate metrics
170+
// with prometheus.DefaultRegisterer and (b) can affect test semantics
171+
// by introducing caching where there was previously a NoopCache.
172+
cfg.DispatchClusterMetricsEnabled = false
173+
cfg.DispatchClientMetricsEnabled = false
174+
cfg.DatastoreConfig.EnableDatastoreMetrics = false
175+
cfg.NamespaceCacheConfig = server.CacheConfig{}
176+
cfg.DispatchCacheConfig = server.CacheConfig{}
177+
cfg.ClusterDispatchCacheConfig = server.CacheConfig{}
178+
cfg.LR3ResourceChunkCacheConfig = server.CacheConfig{}
179+
cfg.StoredSchemaCacheConfig = server.CacheConfig{}
180+
srv, err := cfg.Complete(ctx)
167181
require.NoError(t, err)
168182

169183
go func() {

pkg/cmd/datastore/datastore.go

Lines changed: 62 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@ var BuilderForEngine = map[string]engineBuilderFunc{
5252

5353
//go:generate go run github.com/ecordell/optgen -output zz_generated.connpool.options.go . ConnPoolConfig
5454
type ConnPoolConfig struct {
55-
MaxIdleTime time.Duration `debugmap:"visible"`
56-
MaxLifetime time.Duration `debugmap:"visible"`
55+
MaxIdleTime time.Duration `debugmap:"visible" default:"30m"`
56+
MaxLifetime time.Duration `debugmap:"visible" default:"30m"`
5757
MaxLifetimeJitter time.Duration `debugmap:"visible"`
5858
MaxOpenConns int `debugmap:"visible"`
5959
MinOpenConns int `debugmap:"visible"`
60-
HealthCheckInterval time.Duration `debugmap:"visible"`
60+
HealthCheckInterval time.Duration `debugmap:"visible" default:"30s"`
6161
}
6262

6363
func DefaultReadConnPool() *ConnPoolConfig {
@@ -104,20 +104,20 @@ func deprecateUnifiedConnFlags(flagSet *pflag.FlagSet) {
104104

105105
//go:generate go run github.com/ecordell/optgen -sensitive-field-name-matches uri,secure -output zz_generated.options.go . Config
106106
type Config struct {
107-
Engine string `debugmap:"visible"`
107+
Engine string `debugmap:"visible" default:"memory"`
108108
URI string `debugmap:"sensitive"`
109-
GCWindow time.Duration `debugmap:"visible"`
110-
LegacyFuzzing time.Duration `debugmap:"visible"`
111-
RevisionQuantization time.Duration `debugmap:"visible"`
112-
MaxRevisionStalenessPercent float64 `debugmap:"visible"`
109+
GCWindow time.Duration `debugmap:"visible" default:"24h"`
110+
LegacyFuzzing time.Duration `debugmap:"visible" default:"-1ns"`
111+
RevisionQuantization time.Duration `debugmap:"visible" default:"5s"`
112+
MaxRevisionStalenessPercent float64 `debugmap:"visible" default:"0.1"`
113113
CredentialsProviderName string `debugmap:"visible"`
114114
FilterMaximumIDCount uint16 `debugmap:"hidden" default:"100"`
115115

116116
// Options
117117
ReadConnPool ConnPoolConfig `debugmap:"visible"`
118118
WriteConnPool ConnPoolConfig `debugmap:"visible"`
119119
ReadOnly bool `debugmap:"visible"`
120-
EnableDatastoreMetrics bool `debugmap:"visible"`
120+
EnableDatastoreMetrics bool `debugmap:"visible" default:"true"`
121121
DisableStats bool `debugmap:"visible"`
122122
IncludeQueryParametersInTraces bool `debugmap:"visible"`
123123

@@ -132,7 +132,7 @@ type Config struct {
132132
BootstrapFiles []string `debugmap:"visible-format"`
133133
BootstrapFileContents map[string][]byte `debugmap:"visible"`
134134
BootstrapOverwrite bool `debugmap:"visible"`
135-
BootstrapTimeout time.Duration `debugmap:"visible"`
135+
BootstrapTimeout time.Duration `debugmap:"visible" default:"10s"`
136136
CaveatTypeSet *caveattypes.TypeSet `debugmap:"hidden"`
137137

138138
// Hedging
@@ -142,17 +142,17 @@ type Config struct {
142142
RequestHedgingQuantile float64 `debugmap:"visible"`
143143

144144
// CRDB
145-
FollowerReadDelay time.Duration `debugmap:"visible"`
146-
MaxRetries int `debugmap:"visible"`
147-
OverlapKey string `debugmap:"visible"`
148-
OverlapStrategy string `debugmap:"visible"`
149-
EnableConnectionBalancing bool `debugmap:"visible"`
150-
ConnectRate time.Duration `debugmap:"visible"`
151-
WriteAcquisitionTimeout time.Duration `debugmap:"visible"`
145+
FollowerReadDelay time.Duration `debugmap:"visible" default:"4800ms"`
146+
MaxRetries int `debugmap:"visible" default:"10"`
147+
OverlapKey string `debugmap:"visible" default:"key"`
148+
OverlapStrategy string `debugmap:"visible" default:"static"`
149+
EnableConnectionBalancing bool `debugmap:"visible" default:"true"`
150+
ConnectRate time.Duration `debugmap:"visible" default:"100ms"`
151+
WriteAcquisitionTimeout time.Duration `debugmap:"visible" default:"30ms"`
152152

153153
// Postgres
154-
GCInterval time.Duration `debugmap:"visible"`
155-
GCMaxOperationTime time.Duration `debugmap:"visible"`
154+
GCInterval time.Duration `debugmap:"visible" default:"3m"`
155+
GCMaxOperationTime time.Duration `debugmap:"visible" default:"1m"`
156156
RelaxedIsolationLevel bool `debugmap:"visible"`
157157

158158
// Spanner
@@ -168,9 +168,9 @@ type Config struct {
168168
// https://docs.cloud.google.com/docs/authentication/client-libraries#adc
169169
SpannerCredentialsJSON []byte `debugmap:"sensitive"`
170170
SpannerEmulatorHost string `debugmap:"visible"`
171-
SpannerMinSessions uint64 `debugmap:"visible"`
172-
SpannerMaxSessions uint64 `debugmap:"visible"`
173-
SpannerDatastoreMetricsOption string `debugmap:"visible"`
171+
SpannerMinSessions uint64 `debugmap:"visible" default:"100"`
172+
SpannerMaxSessions uint64 `debugmap:"visible" default:"400"`
173+
SpannerDatastoreMetricsOption string `debugmap:"visible" default:"otel"`
174174

175175
// MySQL
176176
TablePrefix string `debugmap:"visible"`
@@ -181,21 +181,56 @@ type Config struct {
181181
RelationshipIntegrityExpiredKeys []string `debugmap:"visible"`
182182

183183
// Internal
184-
WatchBufferLength uint16 `debugmap:"visible"`
185-
WatchChangeBufferMaximumSize string `debugmap:"visible"`
186-
WatchBufferWriteTimeout time.Duration `debugmap:"visible"`
187-
WatchConnectTimeout time.Duration `debugmap:"visible"`
184+
WatchBufferLength uint16 `debugmap:"visible" default:"1024"`
185+
WatchChangeBufferMaximumSize string `debugmap:"visible" default:"15%"`
186+
WatchBufferWriteTimeout time.Duration `debugmap:"visible" default:"1s"`
187+
WatchConnectTimeout time.Duration `debugmap:"visible" default:"1s"`
188188
DisableWatchSupport bool `debugmap:"hidden"`
189189

190190
// Migrations
191191
MigrationPhase string `debugmap:"visible"`
192192
AllowedMigrations []string `debugmap:"visible"`
193193

194194
// Experimental
195-
ExperimentalColumnOptimization bool `debugmap:"visible"`
195+
ExperimentalColumnOptimization bool `debugmap:"visible" default:"true"`
196196
EnableRevisionHeartbeat bool `debugmap:"visible"`
197197
}
198198

199+
// SetDefaults is invoked by github.com/creasty/defaults after struct-tag
200+
// defaults are applied. It fills the four ConnPoolConfig slots from the
201+
// canonical DefaultReadConnPool / DefaultWriteConnPool constructors because
202+
// each slot receives a different default set from RegisterConnPoolFlagsWithPrefix
203+
// in RegisterDatastoreFlagsWithPrefix (Read pools = 20/20 conns, Write = 10/10).
204+
// It also pre-allocates slice fields to empty (non-nil) values so the
205+
// resulting Config matches what RegisterDatastoreFlags writes via
206+
// StringSliceVar/StringArrayVar.
207+
func (c *Config) SetDefaults() {
208+
c.ReadConnPool = *DefaultReadConnPool()
209+
c.WriteConnPool = *DefaultWriteConnPool()
210+
c.ReadReplicaConnPool = *DefaultReadConnPool()
211+
c.OldReadReplicaConnPool = *DefaultReadConnPool()
212+
213+
// CaveatTypeSet is hidden from DebugMap but RegisterDatastoreFlags
214+
// initializes it from DefaultDatastoreConfig at line 223. Mirror that
215+
// here so library users get the same value as CLI users.
216+
if c.CaveatTypeSet == nil {
217+
c.CaveatTypeSet = caveattypes.Default.TypeSet
218+
}
219+
220+
if c.BootstrapFiles == nil {
221+
c.BootstrapFiles = []string{}
222+
}
223+
if c.ReadReplicaURIs == nil {
224+
c.ReadReplicaURIs = []string{}
225+
}
226+
if c.AllowedMigrations == nil {
227+
c.AllowedMigrations = []string{}
228+
}
229+
if c.RelationshipIntegrityExpiredKeys == nil {
230+
c.RelationshipIntegrityExpiredKeys = []string{}
231+
}
232+
}
233+
199234
//go:generate go run github.com/ecordell/optgen -sensitive-field-name-matches uri,secure -output zz_generated.relintegritykey.options.go . RelIntegrityKey
200235
type RelIntegrityKey struct {
201236
KeyID string `debugmap:"visible"`

pkg/cmd/serve_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,23 @@ func restoreEnv(prevEnv []string) {
7474
}
7575
}
7676

77+
// TestConfigDefaultsMatchCLIFlags asserts that NewConfigWithOptionsAndDefaults
78+
// (the library entry point) produces the same Config as RegisterServeFlags
79+
// (the CLI entry point). Any drift is a bug: a new flag or a new struct field
80+
// added without a matching default tag/SetDefaults hook will be caught here.
81+
//
82+
// pflag writes each flag's default into the bound destination at registration
83+
// time, so RegisterServeFlags alone (no ParseFlags call) is enough to fully
84+
// populate cliCfg with the CLI defaults.
85+
func TestConfigDefaultsMatchCLIFlags(t *testing.T) {
86+
cliCfg := &server.Config{}
87+
require.NoError(t, RegisterServeFlags(&cobra.Command{}, cliCfg))
88+
89+
libCfg := server.NewConfigWithOptionsAndDefaults()
90+
91+
require.Equal(t, cliCfg.DebugMap(), libCfg.DebugMap())
92+
}
93+
7794
func TestDefaultConfig(t *testing.T) {
7895
flags := []string{
7996
"--grpc-preshared-key", "some_key",

pkg/cmd/server/otel.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@ type otelShutdowner interface {
3030
}
3131

3232
type OTelConfig struct {
33-
Provider string `debugmap:"visible"`
33+
Provider string `debugmap:"visible" default:"none"`
3434
Endpoint string `debugmap:"visible"`
35-
ServiceName string `debugmap:"visible"`
36-
TracePropagator string `debugmap:"visible"`
35+
ServiceName string `debugmap:"visible" default:"spicedb"`
36+
TracePropagator string `debugmap:"visible" default:"w3c"`
3737
UsePlaintext bool `debugmap:"visible"`
38-
SampleRatio float64 `debugmap:"visible"`
38+
SampleRatio float64 `debugmap:"visible" default:"0.01"`
3939
}
4040

4141
// RegisterOTelFlags registers all OpenTelemetry flags on cmd, binding them directly into cfg.

0 commit comments

Comments
 (0)