Current Implementation:
type StdConfigProvider struct {
cfg any
}
func (s *StdConfigProvider) GetConfig() any {
return s.cfg // Returns the SAME reference every time
}Problem: All consumers of a config provider receive the same pointer. Modifications by any consumer affect all others.
The framework has deep copy functionality (deepCopyValue, createTempConfigDeep) but:
- These functions are package-private (not exported)
- They're only used for temporary processing during validation
- Not used to protect configs from mutation
Example from reverseproxy/module.go:482-487:
func (m *ReverseProxyModule) validateConfig() error {
// ...
if m.config.CacheEnabled && m.config.CacheTTL <= 0 {
m.app.Logger().Warn("Cache is enabled but CacheTTL is not set, using default of 60s")
m.config.CacheTTL = 60 * time.Second // MODIFIES THE CONFIG IN-PLACE
}
// ...
}Impact: This modification affects the config provider's stored reference, potentially affecting other modules or subsequent test runs.
- Multi-tenant applications: Tenant A's config modifications could bleed into Tenant B
- Module initialization order: Later modules might see modified configs from earlier modules
- Config reloading: Reloading configs might inherit stale modifications
- Test isolation failures: Exactly what we observed - tests pollute each other's configs
- Flaky tests: Tests pass/fail depending on execution order
- Hard to debug: The source of pollution is far from where symptoms appear
Make StdConfigProvider return a copy:
func (s *StdConfigProvider) GetConfig() any {
// Return a deep copy instead of the original reference
copied, _, err := createTempConfigDeep(s.cfg)
if err != nil {
// Fallback to original if copy fails
return s.cfg
}
return copied
}Pros:
- Automatic protection for all configs
- No code changes needed in modules
- Works for existing applications
Cons:
- Performance overhead (copying on every GetConfig call)
- Need to export createTempConfigDeep
- May break applications that rely on shared mutation (though that's a bug)
Make configs immutable after initialization:
type ImmutableConfigProvider struct {
cfg any
frozen bool
}
func (p *ImmutableConfigProvider) GetConfig() any {
if p.frozen {
// Return a deep copy since config is frozen
return deepCopy(p.cfg)
}
return p.cfg
}
func (p *ImmutableConfigProvider) Freeze() {
p.frozen = true
}Pros:
- Clear semantics - configs can't change after freezing
- Performance - only copy when needed
- Explicit control
Cons:
- Requires application code changes
- Modules must be careful not to modify configs
- Backward compatibility issues
Lazy copy when config would be modified:
type CowConfigProvider struct {
original any
copy any
dirty bool
}
func (p *CowConfigProvider) GetConfig() any {
if !p.dirty {
return p.original
}
return p.copy
}
func (p *CowConfigProvider) GetMutableConfig() any {
if !p.dirty {
p.copy = deepCopy(p.original)
p.dirty = true
}
return p.copy
}Pros:
- Performance - only copy when needed
- Explicit intent - GetMutableConfig() vs GetConfig()
- Tracks modification state
Cons:
- Two different methods - API complexity
- Requires module authors to use GetMutableConfig() when modifying
- Still allows shared mutation if everyone uses GetMutableConfig()
Framework copies config before passing to module:
func (app *StdApplication) Init() error {
for _, module := range app.modules {
cfg, err := app.GetConfigSection(module.Name())
if err != nil {
return err
}
// Create a deep copy for this specific module instance
cfgCopy, _, err := createTempConfigDeep(cfg.GetConfig())
if err != nil {
return err
}
// Pass the copy to the module
module.SetConfig(cfgCopy)
}
}Pros:
- Each module gets its own config copy
- No config provider changes
- Isolated module configs
Cons:
- Breaks shared config scenarios (if intentional)
- Module interface changes (need SetConfig method)
- Doesn't solve test isolation (same provider used across tests)
Hybrid: Option 1 + Export Deep Copy Utilities
-
Export deep copy functions:
// Export for use by modules and tests func DeepCopyConfig(cfg any) (any, error) { copied, _, err := createTempConfigDeep(cfg) return copied, err }
-
Add option to StdConfigProvider:
type StdConfigProvider struct { cfg any copyMode ConfigCopyMode } type ConfigCopyMode int const ( NoCopy ConfigCopyMode = iota // Current behavior (reference) DeepCopy // Return deep copy ) func NewStdConfigProvider(cfg any) *StdConfigProvider { return &StdConfigProvider{cfg: cfg, copyMode: NoCopy} } func NewIsolatedConfigProvider(cfg any) *StdConfigProvider { return &StdConfigProvider{cfg: cfg, copyMode: DeepCopy} } func (s *StdConfigProvider) GetConfig() any { if s.copyMode == DeepCopy { copied, _, _ := createTempConfigDeep(s.cfg) return copied } return s.cfg }
-
Best Practices Documentation:
- Tests should use
NewIsolatedConfigProvider - Production apps should consider isolation needs
- Modules should NOT modify configs in-place
- Tests should use
- ✅ Remove test-specific deepCopyConfig - use framework's deep copy
- Export DeepCopyConfig function from config_provider.go
- Add NewIsolatedConfigProvider for test isolation
- Update BDD tests to use NewIsolatedConfigProvider
- Add lint rule to warn about config field mutations
- Document config isolation best practices
- Consider making DeepCopy the default in a future major version
- Export deep copy utilities
- Add NewIsolatedConfigProvider
- Fix tests to use isolated providers
- Document the issue and best practices
- Add deprecation warnings for direct config mutation
- Add metrics to track config provider usage patterns
- Provide migration guide
- Make DeepCopy the default behavior
- Remove NoCopy mode (breaking change)
- Full config immutability guarantee