diff --git a/.mockery.yaml b/.mockery.yaml index 30e36ad..fb6e87d 100644 --- a/.mockery.yaml +++ b/.mockery.yaml @@ -74,3 +74,11 @@ packages: interfaces: ClusterManager: SchemaWatcher: + + github.com/platform-mesh/kubernetes-graphql-gateway/listener/reconciler/kcp: + config: + dir: listener/reconciler/kcp/mocks + outpkg: mocks + interfaces: + VirtualWorkspaceConfigManager: + VirtualWorkspaceConfigReconciler: diff --git a/cmd/listener.go b/cmd/listener.go index dd61782..e6cc162 100644 --- a/cmd/listener.go +++ b/cmd/listener.go @@ -116,6 +116,7 @@ var listenCmd = &cobra.Command{ if appCfg.Listener.VirtualWorkspacesConfigPath != "" { go func() { if err := kcpReconciler.StartVirtualWorkspaceWatching(ctx, appCfg.Listener.VirtualWorkspacesConfigPath); err != nil { + // log.Fatal() calls os.Exit(1)) under the hood and exits the whole application log.Fatal().Err(err).Msg("failed to start virtual workspace watching") } }() diff --git a/listener/reconciler/kcp/config_watcher.go b/listener/reconciler/kcp/config_watcher.go index 4dcf37c..d9f3282 100644 --- a/listener/reconciler/kcp/config_watcher.go +++ b/listener/reconciler/kcp/config_watcher.go @@ -13,18 +13,25 @@ type VirtualWorkspaceConfigManager interface { LoadConfig(configPath string) (*VirtualWorkspacesConfig, error) } +// VirtualWorkspaceConfigReconciler interface for reconciling virtual workspace configurations +type VirtualWorkspaceConfigReconciler interface { + ReconcileConfig(ctx context.Context, config *VirtualWorkspacesConfig) error +} + // ConfigWatcher watches the virtual workspaces configuration file for changes type ConfigWatcher struct { fileWatcher *watcher.FileWatcher virtualWSManager VirtualWorkspaceConfigManager + reconciler VirtualWorkspaceConfigReconciler log *logger.Logger - changeHandler func(*VirtualWorkspacesConfig) + ctx context.Context } // NewConfigWatcher creates a new config file watcher -func NewConfigWatcher(virtualWSManager VirtualWorkspaceConfigManager, log *logger.Logger) (*ConfigWatcher, error) { +func NewConfigWatcher(virtualWSManager VirtualWorkspaceConfigManager, reconciler VirtualWorkspaceConfigReconciler, log *logger.Logger) (*ConfigWatcher, error) { c := &ConfigWatcher{ virtualWSManager: virtualWSManager, + reconciler: reconciler, log: log, } @@ -38,18 +45,18 @@ func NewConfigWatcher(virtualWSManager VirtualWorkspaceConfigManager, log *logge } // Watch starts watching the configuration file and blocks until context is cancelled -func (c *ConfigWatcher) Watch(ctx context.Context, configPath string, changeHandler func(*VirtualWorkspacesConfig)) error { - // Store change handler for use in event callbacks - c.changeHandler = changeHandler - - // Load initial configuration - if configPath != "" { - if err := c.loadAndNotify(configPath); err != nil { - c.log.Error().Err(err).Msg("failed to load initial virtual workspaces config") - } +func (c *ConfigWatcher) Watch(ctx context.Context, configPath string) error { + if configPath == "" { + return nil + } + + c.ctx = ctx + + if err := c.loadAndNotify(configPath); err != nil { + c.log.Error().Err(err).Msg("failed to load initial virtual workspaces config") + return err } - // Watch optional configuration file with 500ms debouncing return c.fileWatcher.WatchOptionalFile(ctx, configPath, 500) } @@ -65,7 +72,7 @@ func (c *ConfigWatcher) OnFileDeleted(filepath string) { c.log.Warn().Str("configPath", filepath).Msg("virtual workspaces config file deleted") } -// loadAndNotify loads the config and notifies the change handler +// loadAndNotify loads the config and reconciles it func (c *ConfigWatcher) loadAndNotify(configPath string) error { config, err := c.virtualWSManager.LoadConfig(configPath) if err != nil { @@ -74,8 +81,9 @@ func (c *ConfigWatcher) loadAndNotify(configPath string) error { c.log.Info().Int("virtualWorkspaces", len(config.VirtualWorkspaces)).Msg("loaded virtual workspaces config") - if c.changeHandler != nil { - c.changeHandler(config) + if err := c.reconciler.ReconcileConfig(c.ctx, config); err != nil { + c.log.Error().Err(err).Msg("failed to reconcile virtual workspaces config") + return err } return nil } diff --git a/listener/reconciler/kcp/config_watcher_test.go b/listener/reconciler/kcp/config_watcher_test.go index 8224406..59a116f 100644 --- a/listener/reconciler/kcp/config_watcher_test.go +++ b/listener/reconciler/kcp/config_watcher_test.go @@ -1,4 +1,4 @@ -package kcp +package kcp_test import ( "context" @@ -7,22 +7,13 @@ import ( "github.com/platform-mesh/golang-commons/logger/testlogger" "github.com/platform-mesh/kubernetes-graphql-gateway/common" + "github.com/platform-mesh/kubernetes-graphql-gateway/listener/reconciler/kcp" + "github.com/platform-mesh/kubernetes-graphql-gateway/listener/reconciler/kcp/mocks" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) -// MockVirtualWorkspaceConfigManager for testing -type MockVirtualWorkspaceConfigManager struct { - LoadConfigFunc func(configPath string) (*VirtualWorkspacesConfig, error) -} - -func (m *MockVirtualWorkspaceConfigManager) LoadConfig(configPath string) (*VirtualWorkspacesConfig, error) { - if m.LoadConfigFunc != nil { - return m.LoadConfigFunc(configPath) - } - return &VirtualWorkspacesConfig{}, nil -} - func TestNewConfigWatcher(t *testing.T) { tests := []struct { name string @@ -36,9 +27,10 @@ func TestNewConfigWatcher(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - virtualWSManager := &MockVirtualWorkspaceConfigManager{} + virtualWSManager := mocks.NewMockVirtualWorkspaceConfigManager(t) + reconciler := mocks.NewMockVirtualWorkspaceConfigReconciler(t) - watcher, err := NewConfigWatcher(virtualWSManager, testlogger.New().Logger) + watcher, err := kcp.NewConfigWatcher(virtualWSManager, reconciler, testlogger.New().Logger) if tt.expectError { assert.Error(t, err) @@ -46,9 +38,6 @@ func TestNewConfigWatcher(t *testing.T) { } else { assert.NoError(t, err) assert.NotNil(t, watcher) - assert.Equal(t, virtualWSManager, watcher.virtualWSManager) - assert.Equal(t, testlogger.New().Logger, watcher.log) - assert.NotNil(t, watcher.fileWatcher) } }) } @@ -56,180 +45,68 @@ func TestNewConfigWatcher(t *testing.T) { func TestConfigWatcher_OnFileChanged(t *testing.T) { tests := []struct { - name string - filepath string - loadConfigFunc func(configPath string) (*VirtualWorkspacesConfig, error) - expectHandlerCall bool + name string + filepath string + setupMocks func(*mocks.MockVirtualWorkspaceConfigManager, *mocks.MockVirtualWorkspaceConfigReconciler) }{ { name: "successful_file_change", filepath: "/test/config.yaml", - loadConfigFunc: func(configPath string) (*VirtualWorkspacesConfig, error) { - return &VirtualWorkspacesConfig{ - VirtualWorkspaces: []VirtualWorkspace{ + setupMocks: func(manager *mocks.MockVirtualWorkspaceConfigManager, reconciler *mocks.MockVirtualWorkspaceConfigReconciler) { + config := &kcp.VirtualWorkspacesConfig{ + VirtualWorkspaces: []kcp.VirtualWorkspace{ {Name: "test-ws", URL: "https://example.com/"}, }, - }, nil + } + manager.EXPECT().LoadConfig("/test/config.yaml").Return(config, nil) + reconciler.EXPECT().ReconcileConfig(mock.Anything, config).Return(nil) }, - expectHandlerCall: true, }, { name: "failed_config_load", filepath: "/test/config.yaml", - loadConfigFunc: func(configPath string) (*VirtualWorkspacesConfig, error) { - return nil, errors.New("failed to load config") + setupMocks: func(manager *mocks.MockVirtualWorkspaceConfigManager, reconciler *mocks.MockVirtualWorkspaceConfigReconciler) { + manager.EXPECT().LoadConfig("/test/config.yaml").Return((*kcp.VirtualWorkspacesConfig)(nil), errors.New("failed to load config")) }, - expectHandlerCall: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - virtualWSManager := &MockVirtualWorkspaceConfigManager{ - LoadConfigFunc: tt.loadConfigFunc, - } + virtualWSManager := mocks.NewMockVirtualWorkspaceConfigManager(t) + reconciler := mocks.NewMockVirtualWorkspaceConfigReconciler(t) - watcher, err := NewConfigWatcher(virtualWSManager, testlogger.New().Logger) - require.NoError(t, err) + tt.setupMocks(virtualWSManager, reconciler) - // Track change handler calls - var handlerCalled bool - var receivedConfig *VirtualWorkspacesConfig - changeHandler := func(config *VirtualWorkspacesConfig) { - handlerCalled = true - receivedConfig = config - } - watcher.changeHandler = changeHandler + watcher, err := kcp.NewConfigWatcher(virtualWSManager, reconciler, testlogger.New().Logger) + require.NoError(t, err) watcher.OnFileChanged(tt.filepath) - - if tt.expectHandlerCall { - assert.True(t, handlerCalled) - assert.NotNil(t, receivedConfig) - assert.Equal(t, 1, len(receivedConfig.VirtualWorkspaces)) - assert.Equal(t, "test-ws", receivedConfig.VirtualWorkspaces[0].Name) - } else { - assert.False(t, handlerCalled) - } }) } } func TestConfigWatcher_OnFileDeleted(t *testing.T) { - virtualWSManager := &MockVirtualWorkspaceConfigManager{} + virtualWSManager := mocks.NewMockVirtualWorkspaceConfigManager(t) + reconciler := mocks.NewMockVirtualWorkspaceConfigReconciler(t) - watcher, err := NewConfigWatcher(virtualWSManager, testlogger.New().Logger) + watcher, err := kcp.NewConfigWatcher(virtualWSManager, reconciler, testlogger.New().Logger) require.NoError(t, err) - // Should not panic or error watcher.OnFileDeleted("/test/config.yaml") } -func TestConfigWatcher_LoadAndNotify(t *testing.T) { - tests := []struct { - name string - configPath string - loadConfigFunc func(configPath string) (*VirtualWorkspacesConfig, error) - expectError bool - expectCall bool - }{ - { - name: "successful_load_and_notify", - configPath: "/test/config.yaml", - loadConfigFunc: func(configPath string) (*VirtualWorkspacesConfig, error) { - return &VirtualWorkspacesConfig{ - VirtualWorkspaces: []VirtualWorkspace{ - {Name: "ws1", URL: "https://example.com/"}, - {Name: "ws2", URL: "https://example.org/"}, - }, - }, nil - }, - expectError: false, - expectCall: true, - }, - { - name: "failed_config_load", - configPath: "/test/config.yaml", - loadConfigFunc: func(configPath string) (*VirtualWorkspacesConfig, error) { - return nil, errors.New("config load error") - }, - expectError: true, - expectCall: false, - }, - { - name: "no_change_handler", - configPath: "/test/config.yaml", - loadConfigFunc: func(configPath string) (*VirtualWorkspacesConfig, error) { - return &VirtualWorkspacesConfig{}, nil - }, - expectError: false, - expectCall: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - virtualWSManager := &MockVirtualWorkspaceConfigManager{ - LoadConfigFunc: tt.loadConfigFunc, - } - - watcher, err := NewConfigWatcher(virtualWSManager, testlogger.New().Logger) - require.NoError(t, err) - - // Track change handler calls - var handlerCalled bool - var receivedConfig *VirtualWorkspacesConfig - if tt.name != "no_change_handler" { - changeHandler := func(config *VirtualWorkspacesConfig) { - handlerCalled = true - receivedConfig = config - } - watcher.changeHandler = changeHandler - } - - err = watcher.loadAndNotify(tt.configPath) - - if tt.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - - if tt.expectCall { - assert.True(t, handlerCalled) - assert.NotNil(t, receivedConfig) - if tt.name == "successful_load_and_notify" { - assert.Equal(t, 2, len(receivedConfig.VirtualWorkspaces)) - } - } else { - assert.False(t, handlerCalled) - } - }) - } -} - func TestConfigWatcher_Watch_EmptyPath(t *testing.T) { - virtualWSManager := &MockVirtualWorkspaceConfigManager{ - LoadConfigFunc: func(configPath string) (*VirtualWorkspacesConfig, error) { - return &VirtualWorkspacesConfig{}, nil - }, - } + virtualWSManager := mocks.NewMockVirtualWorkspaceConfigManager(t) + reconciler := mocks.NewMockVirtualWorkspaceConfigReconciler(t) - watcher, err := NewConfigWatcher(virtualWSManager, testlogger.New().Logger) + watcher, err := kcp.NewConfigWatcher(virtualWSManager, reconciler, testlogger.New().Logger) require.NoError(t, err) ctx, cancel := context.WithTimeout(t.Context(), common.ShortTimeout) defer cancel() - var handlerCalled bool - changeHandler := func(config *VirtualWorkspacesConfig) { - handlerCalled = true - } - - // Test with empty config path - should not try to load initial config - err = watcher.Watch(ctx, "", changeHandler) + err = watcher.Watch(ctx, "") - // Should complete gracefully without error since graceful termination is not an error assert.NoError(t, err) - assert.False(t, handlerCalled) // Should not call handler for empty path initial load } diff --git a/listener/reconciler/kcp/mocks/mock_VirtualWorkspaceConfigManager.go b/listener/reconciler/kcp/mocks/mock_VirtualWorkspaceConfigManager.go new file mode 100644 index 0000000..81d0c5a --- /dev/null +++ b/listener/reconciler/kcp/mocks/mock_VirtualWorkspaceConfigManager.go @@ -0,0 +1,93 @@ +// Code generated by mockery v2.52.3. DO NOT EDIT. + +package mocks + +import ( + kcp "github.com/platform-mesh/kubernetes-graphql-gateway/listener/reconciler/kcp" + mock "github.com/stretchr/testify/mock" +) + +// MockVirtualWorkspaceConfigManager is an autogenerated mock type for the VirtualWorkspaceConfigManager type +type MockVirtualWorkspaceConfigManager struct { + mock.Mock +} + +type MockVirtualWorkspaceConfigManager_Expecter struct { + mock *mock.Mock +} + +func (_m *MockVirtualWorkspaceConfigManager) EXPECT() *MockVirtualWorkspaceConfigManager_Expecter { + return &MockVirtualWorkspaceConfigManager_Expecter{mock: &_m.Mock} +} + +// LoadConfig provides a mock function with given fields: configPath +func (_m *MockVirtualWorkspaceConfigManager) LoadConfig(configPath string) (*kcp.VirtualWorkspacesConfig, error) { + ret := _m.Called(configPath) + + if len(ret) == 0 { + panic("no return value specified for LoadConfig") + } + + var r0 *kcp.VirtualWorkspacesConfig + var r1 error + if rf, ok := ret.Get(0).(func(string) (*kcp.VirtualWorkspacesConfig, error)); ok { + return rf(configPath) + } + if rf, ok := ret.Get(0).(func(string) *kcp.VirtualWorkspacesConfig); ok { + r0 = rf(configPath) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*kcp.VirtualWorkspacesConfig) + } + } + + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(configPath) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockVirtualWorkspaceConfigManager_LoadConfig_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'LoadConfig' +type MockVirtualWorkspaceConfigManager_LoadConfig_Call struct { + *mock.Call +} + +// LoadConfig is a helper method to define mock.On call +// - configPath string +func (_e *MockVirtualWorkspaceConfigManager_Expecter) LoadConfig(configPath interface{}) *MockVirtualWorkspaceConfigManager_LoadConfig_Call { + return &MockVirtualWorkspaceConfigManager_LoadConfig_Call{Call: _e.mock.On("LoadConfig", configPath)} +} + +func (_c *MockVirtualWorkspaceConfigManager_LoadConfig_Call) Run(run func(configPath string)) *MockVirtualWorkspaceConfigManager_LoadConfig_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(string)) + }) + return _c +} + +func (_c *MockVirtualWorkspaceConfigManager_LoadConfig_Call) Return(_a0 *kcp.VirtualWorkspacesConfig, _a1 error) *MockVirtualWorkspaceConfigManager_LoadConfig_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockVirtualWorkspaceConfigManager_LoadConfig_Call) RunAndReturn(run func(string) (*kcp.VirtualWorkspacesConfig, error)) *MockVirtualWorkspaceConfigManager_LoadConfig_Call { + _c.Call.Return(run) + return _c +} + +// NewMockVirtualWorkspaceConfigManager creates a new instance of MockVirtualWorkspaceConfigManager. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewMockVirtualWorkspaceConfigManager(t interface { + mock.TestingT + Cleanup(func()) +}) *MockVirtualWorkspaceConfigManager { + mock := &MockVirtualWorkspaceConfigManager{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/listener/reconciler/kcp/mocks/mock_VirtualWorkspaceConfigReconciler.go b/listener/reconciler/kcp/mocks/mock_VirtualWorkspaceConfigReconciler.go new file mode 100644 index 0000000..5deb7ba --- /dev/null +++ b/listener/reconciler/kcp/mocks/mock_VirtualWorkspaceConfigReconciler.go @@ -0,0 +1,84 @@ +// Code generated by mockery v2.52.3. DO NOT EDIT. + +package mocks + +import ( + context "context" + + kcp "github.com/platform-mesh/kubernetes-graphql-gateway/listener/reconciler/kcp" + mock "github.com/stretchr/testify/mock" +) + +// MockVirtualWorkspaceConfigReconciler is an autogenerated mock type for the VirtualWorkspaceConfigReconciler type +type MockVirtualWorkspaceConfigReconciler struct { + mock.Mock +} + +type MockVirtualWorkspaceConfigReconciler_Expecter struct { + mock *mock.Mock +} + +func (_m *MockVirtualWorkspaceConfigReconciler) EXPECT() *MockVirtualWorkspaceConfigReconciler_Expecter { + return &MockVirtualWorkspaceConfigReconciler_Expecter{mock: &_m.Mock} +} + +// ReconcileConfig provides a mock function with given fields: ctx, config +func (_m *MockVirtualWorkspaceConfigReconciler) ReconcileConfig(ctx context.Context, config *kcp.VirtualWorkspacesConfig) error { + ret := _m.Called(ctx, config) + + if len(ret) == 0 { + panic("no return value specified for ReconcileConfig") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *kcp.VirtualWorkspacesConfig) error); ok { + r0 = rf(ctx, config) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MockVirtualWorkspaceConfigReconciler_ReconcileConfig_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ReconcileConfig' +type MockVirtualWorkspaceConfigReconciler_ReconcileConfig_Call struct { + *mock.Call +} + +// ReconcileConfig is a helper method to define mock.On call +// - ctx context.Context +// - config *kcp.VirtualWorkspacesConfig +func (_e *MockVirtualWorkspaceConfigReconciler_Expecter) ReconcileConfig(ctx interface{}, config interface{}) *MockVirtualWorkspaceConfigReconciler_ReconcileConfig_Call { + return &MockVirtualWorkspaceConfigReconciler_ReconcileConfig_Call{Call: _e.mock.On("ReconcileConfig", ctx, config)} +} + +func (_c *MockVirtualWorkspaceConfigReconciler_ReconcileConfig_Call) Run(run func(ctx context.Context, config *kcp.VirtualWorkspacesConfig)) *MockVirtualWorkspaceConfigReconciler_ReconcileConfig_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*kcp.VirtualWorkspacesConfig)) + }) + return _c +} + +func (_c *MockVirtualWorkspaceConfigReconciler_ReconcileConfig_Call) Return(_a0 error) *MockVirtualWorkspaceConfigReconciler_ReconcileConfig_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockVirtualWorkspaceConfigReconciler_ReconcileConfig_Call) RunAndReturn(run func(context.Context, *kcp.VirtualWorkspacesConfig) error) *MockVirtualWorkspaceConfigReconciler_ReconcileConfig_Call { + _c.Call.Return(run) + return _c +} + +// NewMockVirtualWorkspaceConfigReconciler creates a new instance of MockVirtualWorkspaceConfigReconciler. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewMockVirtualWorkspaceConfigReconciler(t interface { + mock.TestingT + Cleanup(func()) +}) *MockVirtualWorkspaceConfigReconciler { + mock := &MockVirtualWorkspaceConfigReconciler{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/listener/reconciler/kcp/reconciler.go b/listener/reconciler/kcp/reconciler.go index c4c7ae6..0f3d879 100644 --- a/listener/reconciler/kcp/reconciler.go +++ b/listener/reconciler/kcp/reconciler.go @@ -81,7 +81,7 @@ func NewKCPReconciler( log, ) - configWatcher, err := NewConfigWatcher(virtualWSManager, log) + configWatcher, err := NewConfigWatcher(virtualWSManager, virtualWorkspaceReconciler, log) if err != nil { log.Error().Err(err).Msg("failed to create config watcher") return nil, err @@ -137,11 +137,5 @@ func (r *KCPReconciler) StartVirtualWorkspaceWatching(ctx context.Context, confi r.log.Info().Str("configPath", configPath).Msg("starting virtual workspace configuration watching") - // Start config watcher with a wrapper function - changeHandler := func(config *VirtualWorkspacesConfig) { - if err := r.virtualWorkspaceReconciler.ReconcileConfig(ctx, config); err != nil { - r.log.Error().Err(err).Msg("failed to reconcile virtual workspaces config") - } - } - return r.configWatcher.Watch(ctx, configPath, changeHandler) + return r.configWatcher.Watch(ctx, configPath) } diff --git a/listener/reconciler/kcp/virtual_workspace.go b/listener/reconciler/kcp/virtual_workspace.go index 36a92d3..1ab5354 100644 --- a/listener/reconciler/kcp/virtual_workspace.go +++ b/listener/reconciler/kcp/virtual_workspace.go @@ -188,7 +188,7 @@ func (r *VirtualWorkspaceReconciler) ReconcileConfig(ctx context.Context, config if err := r.processVirtualWorkspace(ctx, workspace); err != nil { r.log.Error().Err(err).Str("workspace", name).Msg("failed to process virtual workspace") - continue + return err } } }