Skip to content

fix(core): preserve backward-compatible Loader.Watch interface #3313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pflynn-virtru wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(core): preserve backward-compatible Loader.Watch interface #3313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a2731ef
fix(core): preserve backward-compatible Loader.Watch interface
pflynn-virtru Apr 16, 2026
54b2b92
refactor(core): drop unused NamespaceAwareLoader interface
pflynn-virtru Apr 16, 2026
5a24d3b
Revert "refactor(core): drop unused NamespaceAwareLoader interface"
pflynn-virtru Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion service/pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Comment on lines +184 to +185
Copy link
Copy Markdown
Contributor

@ntrevino-virtru ntrevino-virtru Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be else if

return err
}
}
Expand Down
2 changes: 1 addition & 1 deletion service/pkg/config/config_file_loader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 24 additions & 7 deletions service/pkg/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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",
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion service/pkg/config/default_settings_loader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion service/pkg/config/environment_value_loader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion service/pkg/config/legacy_loader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 11 additions & 3 deletions service/pkg/config/loader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading