From a2731efd5ac03a3e6390f23e9564e7352c2849fa Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Thu, 16 Apr 2026 12:42:42 -0400 Subject: [PATCH 1/3] fix(core): preserve backward-compatible Loader.Watch interface The Loader.Watch method signature was changed to include a namespaces parameter, breaking downstream consumers that implement the interface. Revert Watch to the original signature and introduce a separate NamespaceAwareLoader interface for loaders that need namespace info. Co-Authored-By: Claude Opus 4.6 (1M context) --- service/pkg/config/config.go | 8 ++++- service/pkg/config/config_file_loader.go | 2 +- service/pkg/config/config_test.go | 31 ++++++++++++++----- service/pkg/config/default_settings_loader.go | 2 +- .../pkg/config/environment_value_loader.go | 2 +- service/pkg/config/legacy_loader.go | 2 +- service/pkg/config/loader.go | 14 +++++++-- 7 files changed, 46 insertions(+), 15 deletions(-) diff --git a/service/pkg/config/config.go b/service/pkg/config/config.go index aac79e7fbc..15e7d782bb 100644 --- a/service/pkg/config/config.go +++ b/service/pkg/config/config.go @@ -176,7 +176,13 @@ func (c *Config) WatchWithNamespaces(ctx context.Context, namespaces []Namespace // Now call the user-provided hooks with the new configuration. return c.OnChange(ctx) } - if err := loader.Watch(ctx, c, onChangeCallback, namespaces); err != nil { + if nsLoader, ok := loader.(NamespaceAwareLoader); ok { + if err := nsLoader.WatchWithNamespaces(ctx, c, onChangeCallback, namespaces); err != nil { + return err + } + continue + } + if err := loader.Watch(ctx, c, onChangeCallback); err != nil { return err } } diff --git a/service/pkg/config/config_file_loader.go b/service/pkg/config/config_file_loader.go index aec09c4937..06db0be3fa 100644 --- a/service/pkg/config/config_file_loader.go +++ b/service/pkg/config/config_file_loader.go @@ -63,7 +63,7 @@ func (l *FileLoader) Load(_ Config) error { } // Watch starts watching the config file for configuration changes -func (l *FileLoader) Watch(ctx context.Context, _ *Config, onChange func(context.Context) error, _ []NamespaceInfo) error { +func (l *FileLoader) Watch(ctx context.Context, _ *Config, onChange func(context.Context) error) error { l.viper.WatchConfig() // If config changes, trigger the main config reload function diff --git a/service/pkg/config/config_test.go b/service/pkg/config/config_test.go index 4ce51adc42..b5704a680b 100644 --- a/service/pkg/config/config_test.go +++ b/service/pkg/config/config_test.go @@ -19,7 +19,7 @@ type MockLoader struct { loadFn func(Config) error getFn func(string) (any, error) getConfigKeysFn func() ([]string, error) - watchFn func(context.Context, *Config, func(context.Context) error, []NamespaceInfo) error + watchFn func(context.Context, *Config, func(context.Context) error) error closeFn func() error getNameFn func() string @@ -30,6 +30,23 @@ type MockLoader struct { onChangeCalled bool } +// MockNamespaceAwareLoader extends MockLoader with NamespaceAwareLoader support +type MockNamespaceAwareLoader struct { + MockLoader + watchWithNamespacesFn func(context.Context, *Config, func(context.Context) error, []NamespaceInfo) error +} + +func (l *MockNamespaceAwareLoader) WatchWithNamespaces(ctx context.Context, config *Config, onChange func(context.Context) error, namespaces []NamespaceInfo) error { + l.watchCalled = true + if l.watchWithNamespacesFn != nil { + if err := l.watchWithNamespacesFn(ctx, config, onChange, namespaces); err != nil { + return err + } + l.onChangeCalled = true + } + return nil +} + func (l *MockLoader) Load(mostRecentConfig Config) error { if l.loadFn != nil { return l.loadFn(mostRecentConfig) @@ -51,10 +68,10 @@ func (l *MockLoader) GetConfigKeys() ([]string, error) { return nil, nil } -func (l *MockLoader) Watch(ctx context.Context, config *Config, onChange func(context.Context) error, namespaces []NamespaceInfo) error { +func (l *MockLoader) Watch(ctx context.Context, config *Config, onChange func(context.Context) error) error { l.watchCalled = true if l.watchFn != nil { - if err := l.watchFn(ctx, config, onChange, namespaces); err != nil { + if err := l.watchFn(ctx, config, onChange); err != nil { return err } l.onChangeCalled = true @@ -123,7 +140,7 @@ func TestConfig_Watch(t *testing.T) { config := &Config{} loader := newMockLoader() // Mock loader to call onChange - loader.watchFn = func(_ context.Context, _ *Config, _ func(context.Context) error, _ []NamespaceInfo) error { + loader.watchFn = func(_ context.Context, _ *Config, _ func(context.Context) error) error { return nil } config.AddLoader(loader) @@ -139,7 +156,7 @@ func TestConfig_Watch(t *testing.T) { config := &Config{} expectedErr := errors.New("watch error") loader := newMockLoader() - loader.watchFn = func(_ context.Context, _ *Config, _ func(context.Context) error, _ []NamespaceInfo) error { + loader.watchFn = func(_ context.Context, _ *Config, _ func(context.Context) error) error { return expectedErr } config.AddLoader(loader) @@ -153,7 +170,7 @@ func TestConfig_Watch(t *testing.T) { t.Run("Loader receives NamespaceInfo", func(t *testing.T) { config := &Config{} - loader := newMockLoader() + loader := &MockNamespaceAwareLoader{} testNamespaces := []NamespaceInfo{ { Name: "test-ns", @@ -165,7 +182,7 @@ func TestConfig_Watch(t *testing.T) { } var receivedNamespaces []NamespaceInfo - loader.watchFn = func(_ context.Context, _ *Config, _ func(context.Context) error, namespaces []NamespaceInfo) error { + loader.watchWithNamespacesFn = func(_ context.Context, _ *Config, _ func(context.Context) error, namespaces []NamespaceInfo) error { receivedNamespaces = namespaces return nil } diff --git a/service/pkg/config/default_settings_loader.go b/service/pkg/config/default_settings_loader.go index 9d93a46ce9..392959d802 100644 --- a/service/pkg/config/default_settings_loader.go +++ b/service/pkg/config/default_settings_loader.go @@ -90,7 +90,7 @@ func (l *DefaultSettingsLoader) Load(_ Config) error { return nil } -func (l *DefaultSettingsLoader) Watch(_ context.Context, _ *Config, _ func(context.Context) error, _ []NamespaceInfo) error { +func (l *DefaultSettingsLoader) Watch(_ context.Context, _ *Config, _ func(context.Context) error) error { return nil } diff --git a/service/pkg/config/environment_value_loader.go b/service/pkg/config/environment_value_loader.go index 741fb97d8b..d2821362c7 100644 --- a/service/pkg/config/environment_value_loader.go +++ b/service/pkg/config/environment_value_loader.go @@ -95,7 +95,7 @@ func (l *EnvironmentValueLoader) Load(_ Config) error { } // Watch starts watching the config file for configuration changes -func (l *EnvironmentValueLoader) Watch(_ context.Context, _ *Config, _ func(context.Context) error, _ []NamespaceInfo) error { +func (l *EnvironmentValueLoader) Watch(_ context.Context, _ *Config, _ func(context.Context) error) error { // Environment variables can't be watched. return nil } diff --git a/service/pkg/config/legacy_loader.go b/service/pkg/config/legacy_loader.go index a9c2ff5a50..6c5f651a4f 100644 --- a/service/pkg/config/legacy_loader.go +++ b/service/pkg/config/legacy_loader.go @@ -75,7 +75,7 @@ func (l *LegacyLoader) Load(cfg Config) error { } // Watch starts watching the config file for configuration changes -func (l *LegacyLoader) Watch(ctx context.Context, _ *Config, onChange func(context.Context) error, _ []NamespaceInfo) error { +func (l *LegacyLoader) Watch(ctx context.Context, _ *Config, onChange func(context.Context) error) error { l.viper.WatchConfig() // If config changes, trigger the main config reload function diff --git a/service/pkg/config/loader.go b/service/pkg/config/loader.go index 14ac7bc0ea..cbd13c33d9 100644 --- a/service/pkg/config/loader.go +++ b/service/pkg/config/loader.go @@ -26,11 +26,19 @@ type Loader interface { // Load is called to load/refresh the configuration from its source Load(mostRecentConfig Config) error // Watch starts watching for configuration changes and invokes an onChange callback. - // It receives information about the registered namespaces and services to determine - // if watching is required for this loader. - Watch(ctx context.Context, cfg *Config, onChange func(context.Context) error, namespaces []NamespaceInfo) error + Watch(ctx context.Context, cfg *Config, onChange func(context.Context) error) error // Close closes the configuration loader Close() error // Name returns the name of the configuration loader Name() string } + +// NamespaceAwareLoader extends Loader with namespace-aware watching. +// Loaders that need information about registered namespaces and services +// should implement this interface. +type NamespaceAwareLoader interface { + Loader + // WatchWithNamespaces starts watching for configuration changes with + // information about the registered namespaces and services. + WatchWithNamespaces(ctx context.Context, cfg *Config, onChange func(context.Context) error, namespaces []NamespaceInfo) error +} From 54b2b92e7ba9bc27d9572513940dba5c8f69a21f Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Thu, 16 Apr 2026 12:56:51 -0400 Subject: [PATCH 2/3] refactor(core): drop unused NamespaceAwareLoader interface No in-tree loader implemented NamespaceAwareLoader, and the namespace info plumbed through WatchWithNamespaces was never consumed. Remove the interface, NamespaceInfo/ServiceInfo types, and the WatchWithNamespaces entry point; callers now use the simpler Loader.Watch path directly. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Paul Flynn --- service/pkg/config/config.go | 15 ++--------- service/pkg/config/config_test.go | 43 ------------------------------- service/pkg/config/loader.go | 23 ----------------- service/pkg/server/start.go | 18 +------------ 4 files changed, 3 insertions(+), 96 deletions(-) diff --git a/service/pkg/config/config.go b/service/pkg/config/config.go index 15e7d782bb..b3bcdff64d 100644 --- a/service/pkg/config/config.go +++ b/service/pkg/config/config.go @@ -155,8 +155,8 @@ func (c *Config) AddOnConfigChangeHook(hook ChangeHook) { c.onConfigChangeHooks = append(c.onConfigChangeHooks, hook) } -// WatchWithNamespaces starts watching the configuration for changes in all config loaders, and provides the namespace info to the loaders. -func (c *Config) WatchWithNamespaces(ctx context.Context, namespaces []NamespaceInfo) error { +// Watch starts watching the configuration for changes in all config loaders. +func (c *Config) Watch(ctx context.Context) error { if len(c.loaders) == 0 { return nil } @@ -176,12 +176,6 @@ func (c *Config) WatchWithNamespaces(ctx context.Context, namespaces []Namespace // Now call the user-provided hooks with the new configuration. return c.OnChange(ctx) } - if nsLoader, ok := loader.(NamespaceAwareLoader); ok { - if err := nsLoader.WatchWithNamespaces(ctx, c, onChangeCallback, namespaces); err != nil { - return err - } - continue - } if err := loader.Watch(ctx, c, onChangeCallback); err != nil { return err } @@ -189,11 +183,6 @@ func (c *Config) WatchWithNamespaces(ctx context.Context, namespaces []Namespace return nil } -// Watch starts watching the configuration for changes in all config loaders. -func (c *Config) Watch(ctx context.Context) error { - return c.WatchWithNamespaces(ctx, []NamespaceInfo{}) -} - // Close invokes close method on all config loaders. func (c *Config) Close(ctx context.Context) error { if len(c.loaders) == 0 { diff --git a/service/pkg/config/config_test.go b/service/pkg/config/config_test.go index b5704a680b..5fdeb191bf 100644 --- a/service/pkg/config/config_test.go +++ b/service/pkg/config/config_test.go @@ -30,23 +30,6 @@ type MockLoader struct { onChangeCalled bool } -// MockNamespaceAwareLoader extends MockLoader with NamespaceAwareLoader support -type MockNamespaceAwareLoader struct { - MockLoader - watchWithNamespacesFn func(context.Context, *Config, func(context.Context) error, []NamespaceInfo) error -} - -func (l *MockNamespaceAwareLoader) WatchWithNamespaces(ctx context.Context, config *Config, onChange func(context.Context) error, namespaces []NamespaceInfo) error { - l.watchCalled = true - if l.watchWithNamespacesFn != nil { - if err := l.watchWithNamespacesFn(ctx, config, onChange, namespaces); err != nil { - return err - } - l.onChangeCalled = true - } - return nil -} - func (l *MockLoader) Load(mostRecentConfig Config) error { if l.loadFn != nil { return l.loadFn(mostRecentConfig) @@ -167,32 +150,6 @@ func TestConfig_Watch(t *testing.T) { assert.True(t, loader.watchCalled) assert.False(t, loader.onChangeCalled) }) - - t.Run("Loader receives NamespaceInfo", func(t *testing.T) { - config := &Config{} - loader := &MockNamespaceAwareLoader{} - testNamespaces := []NamespaceInfo{ - { - Name: "test-ns", - Enabled: true, - Services: []ServiceInfo{ - {Namespace: "test-ns", Name: "test-svc"}, - }, - }, - } - - var receivedNamespaces []NamespaceInfo - loader.watchWithNamespacesFn = func(_ context.Context, _ *Config, _ func(context.Context) error, namespaces []NamespaceInfo) error { - receivedNamespaces = namespaces - return nil - } - config.AddLoader(loader) - - err := config.WatchWithNamespaces(ctx, testNamespaces) - - require.NoError(t, err) - assert.Equal(t, testNamespaces, receivedNamespaces) - }) } func TestConfig_Close(t *testing.T) { diff --git a/service/pkg/config/loader.go b/service/pkg/config/loader.go index cbd13c33d9..996393d2f5 100644 --- a/service/pkg/config/loader.go +++ b/service/pkg/config/loader.go @@ -4,19 +4,6 @@ import ( "context" ) -// ServiceInfo represents minimal information about a service for configuration loaders -type ServiceInfo struct { - Namespace string - Name string -} - -// NamespaceInfo represents minimal information about a namespace for configuration loaders -type NamespaceInfo struct { - Name string - Enabled bool - Services []ServiceInfo -} - // Loader defines the interface for loading and managing configuration type Loader interface { // Get fetches a particular config value by dot-delimited key @@ -32,13 +19,3 @@ type Loader interface { // Name returns the name of the configuration loader Name() string } - -// NamespaceAwareLoader extends Loader with namespace-aware watching. -// Loaders that need information about registered namespaces and services -// should implement this interface. -type NamespaceAwareLoader interface { - Loader - // WatchWithNamespaces starts watching for configuration changes with - // information about the registered namespaces and services. - WatchWithNamespaces(ctx context.Context, cfg *Config, onChange func(context.Context) error, namespaces []NamespaceInfo) error -} diff --git a/service/pkg/server/start.go b/service/pkg/server/start.go index f3513a090d..75a080d8d9 100644 --- a/service/pkg/server/start.go +++ b/service/pkg/server/start.go @@ -299,23 +299,7 @@ func Start(f ...StartOptions) error { defer gatewayCleanup() // Start watching the configuration for changes with registered config change service hooks - var watchInfo []config.NamespaceInfo - for _, nsInfo := range svcRegistry.GetNamespaces() { - var services []config.ServiceInfo - for _, svc := range nsInfo.Namespace.Services { - services = append(services, config.ServiceInfo{ - Namespace: svc.GetNamespace(), - Name: svc.GetServiceDesc().ServiceName, - }) - } - watchInfo = append(watchInfo, config.NamespaceInfo{ - Name: nsInfo.Name, - Enabled: nsInfo.Namespace.IsEnabled(cfg.Mode), - Services: services, - }) - } - - if err := cfg.WatchWithNamespaces(ctx, watchInfo); err != nil { + if err := cfg.Watch(ctx); err != nil { return fmt.Errorf("failed to watch configuration: %w", err) } defer cfg.Close(ctx) From 5a24d3b86217cbe4cfdad1bf256576e0aafe118f Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Thu, 16 Apr 2026 12:59:15 -0400 Subject: [PATCH 3/3] Revert "refactor(core): drop unused NamespaceAwareLoader interface" This reverts commit 54b2b92e7ba9bc27d9572513940dba5c8f69a21f. --- service/pkg/config/config.go | 15 +++++++++-- service/pkg/config/config_test.go | 43 +++++++++++++++++++++++++++++++ service/pkg/config/loader.go | 23 +++++++++++++++++ service/pkg/server/start.go | 18 ++++++++++++- 4 files changed, 96 insertions(+), 3 deletions(-) diff --git a/service/pkg/config/config.go b/service/pkg/config/config.go index b3bcdff64d..15e7d782bb 100644 --- a/service/pkg/config/config.go +++ b/service/pkg/config/config.go @@ -155,8 +155,8 @@ func (c *Config) AddOnConfigChangeHook(hook ChangeHook) { c.onConfigChangeHooks = append(c.onConfigChangeHooks, hook) } -// Watch starts watching the configuration for changes in all config loaders. -func (c *Config) Watch(ctx context.Context) error { +// WatchWithNamespaces starts watching the configuration for changes in all config loaders, and provides the namespace info to the loaders. +func (c *Config) WatchWithNamespaces(ctx context.Context, namespaces []NamespaceInfo) error { if len(c.loaders) == 0 { return nil } @@ -176,6 +176,12 @@ func (c *Config) Watch(ctx context.Context) error { // Now call the user-provided hooks with the new configuration. return c.OnChange(ctx) } + if nsLoader, ok := loader.(NamespaceAwareLoader); ok { + if err := nsLoader.WatchWithNamespaces(ctx, c, onChangeCallback, namespaces); err != nil { + return err + } + continue + } if err := loader.Watch(ctx, c, onChangeCallback); err != nil { return err } @@ -183,6 +189,11 @@ func (c *Config) Watch(ctx context.Context) error { return nil } +// Watch starts watching the configuration for changes in all config loaders. +func (c *Config) Watch(ctx context.Context) error { + return c.WatchWithNamespaces(ctx, []NamespaceInfo{}) +} + // Close invokes close method on all config loaders. func (c *Config) Close(ctx context.Context) error { if len(c.loaders) == 0 { diff --git a/service/pkg/config/config_test.go b/service/pkg/config/config_test.go index 5fdeb191bf..b5704a680b 100644 --- a/service/pkg/config/config_test.go +++ b/service/pkg/config/config_test.go @@ -30,6 +30,23 @@ type MockLoader struct { onChangeCalled bool } +// MockNamespaceAwareLoader extends MockLoader with NamespaceAwareLoader support +type MockNamespaceAwareLoader struct { + MockLoader + watchWithNamespacesFn func(context.Context, *Config, func(context.Context) error, []NamespaceInfo) error +} + +func (l *MockNamespaceAwareLoader) WatchWithNamespaces(ctx context.Context, config *Config, onChange func(context.Context) error, namespaces []NamespaceInfo) error { + l.watchCalled = true + if l.watchWithNamespacesFn != nil { + if err := l.watchWithNamespacesFn(ctx, config, onChange, namespaces); err != nil { + return err + } + l.onChangeCalled = true + } + return nil +} + func (l *MockLoader) Load(mostRecentConfig Config) error { if l.loadFn != nil { return l.loadFn(mostRecentConfig) @@ -150,6 +167,32 @@ func TestConfig_Watch(t *testing.T) { assert.True(t, loader.watchCalled) assert.False(t, loader.onChangeCalled) }) + + t.Run("Loader receives NamespaceInfo", func(t *testing.T) { + config := &Config{} + loader := &MockNamespaceAwareLoader{} + testNamespaces := []NamespaceInfo{ + { + Name: "test-ns", + Enabled: true, + Services: []ServiceInfo{ + {Namespace: "test-ns", Name: "test-svc"}, + }, + }, + } + + var receivedNamespaces []NamespaceInfo + loader.watchWithNamespacesFn = func(_ context.Context, _ *Config, _ func(context.Context) error, namespaces []NamespaceInfo) error { + receivedNamespaces = namespaces + return nil + } + config.AddLoader(loader) + + err := config.WatchWithNamespaces(ctx, testNamespaces) + + require.NoError(t, err) + assert.Equal(t, testNamespaces, receivedNamespaces) + }) } func TestConfig_Close(t *testing.T) { diff --git a/service/pkg/config/loader.go b/service/pkg/config/loader.go index 996393d2f5..cbd13c33d9 100644 --- a/service/pkg/config/loader.go +++ b/service/pkg/config/loader.go @@ -4,6 +4,19 @@ import ( "context" ) +// ServiceInfo represents minimal information about a service for configuration loaders +type ServiceInfo struct { + Namespace string + Name string +} + +// NamespaceInfo represents minimal information about a namespace for configuration loaders +type NamespaceInfo struct { + Name string + Enabled bool + Services []ServiceInfo +} + // Loader defines the interface for loading and managing configuration type Loader interface { // Get fetches a particular config value by dot-delimited key @@ -19,3 +32,13 @@ type Loader interface { // Name returns the name of the configuration loader Name() string } + +// NamespaceAwareLoader extends Loader with namespace-aware watching. +// Loaders that need information about registered namespaces and services +// should implement this interface. +type NamespaceAwareLoader interface { + Loader + // WatchWithNamespaces starts watching for configuration changes with + // information about the registered namespaces and services. + WatchWithNamespaces(ctx context.Context, cfg *Config, onChange func(context.Context) error, namespaces []NamespaceInfo) error +} diff --git a/service/pkg/server/start.go b/service/pkg/server/start.go index 75a080d8d9..f3513a090d 100644 --- a/service/pkg/server/start.go +++ b/service/pkg/server/start.go @@ -299,7 +299,23 @@ func Start(f ...StartOptions) error { defer gatewayCleanup() // Start watching the configuration for changes with registered config change service hooks - if err := cfg.Watch(ctx); err != nil { + var watchInfo []config.NamespaceInfo + for _, nsInfo := range svcRegistry.GetNamespaces() { + var services []config.ServiceInfo + for _, svc := range nsInfo.Namespace.Services { + services = append(services, config.ServiceInfo{ + Namespace: svc.GetNamespace(), + Name: svc.GetServiceDesc().ServiceName, + }) + } + watchInfo = append(watchInfo, config.NamespaceInfo{ + Name: nsInfo.Name, + Enabled: nsInfo.Namespace.IsEnabled(cfg.Mode), + Services: services, + }) + } + + if err := cfg.WatchWithNamespaces(ctx, watchInfo); err != nil { return fmt.Errorf("failed to watch configuration: %w", err) } defer cfg.Close(ctx)