Skip to content

Commit 54c37d1

Browse files
claudeclaude
committed
Add complete settings management with restart warnings
Extend the configuration management system to support all settings via UI/API/CLI. Add restart warning mechanism that alerts users when they change settings that require a service restart to take effect. Backend changes: - Extend ConfigProjection with all settings: server, provision, cloning, pool manager, observer, diagnostic, embedded UI, platform, webhooks - Add restart tag to fields that require restart (server.port, provision.portPool, poolManager.mountDir, embeddedUI settings) - Create ConfigUpdateResponse model with warnings, requiresRestart, changedSettings, and restartSettings fields - Add detectConfigChanges function to compare old and new configs - Add generateRestartWarnings to create warning messages - Add validation functions for new settings (port pool, diagnostic, embedded UI, webhooks) - Allow non-logical mode settings to be modified without retrieval check Frontend changes: - Update configTypes with all new settings sections - Add ConfigWarning and ConfigUpdateResponse types - Extend useForm with all new form fields and validation - Add tooltips for new settings with restart warnings - Update Configuration page to display restart warnings on submit - Update Main store to track configWarnings
1 parent 4df2616 commit 54c37d1

File tree

9 files changed

+955
-79
lines changed

9 files changed

+955
-79
lines changed

‎engine/internal/srv/config.go‎

Lines changed: 261 additions & 44 deletions
Large diffs are not rendered by default.

‎engine/internal/srv/config_test.go‎

Lines changed: 226 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,42 +4,254 @@ import (
44
"testing"
55

66
"github.com/stretchr/testify/require"
7+
8+
"gitlab.com/postgres-ai/database-lab/v3/pkg/models"
79
)
810

911
func TestCustomOptions(t *testing.T) {
1012
testCases := []struct {
1113
customOptions []interface{}
1214
expectedResult error
15+
}{
16+
{customOptions: []interface{}{"--verbose"}, expectedResult: nil},
17+
{customOptions: []interface{}{"--exclude-scheme=test_scheme"}, expectedResult: nil},
18+
{customOptions: []interface{}{`--exclude-scheme="test_scheme"`}, expectedResult: nil},
19+
{customOptions: []interface{}{"--table=$(echo 'test')"}, expectedResult: errInvalidOption},
20+
{customOptions: []interface{}{"--table=test&table"}, expectedResult: errInvalidOption},
21+
{customOptions: []interface{}{5}, expectedResult: errInvalidOptionType},
22+
}
23+
24+
for _, tc := range testCases {
25+
validationResult := validateCustomOptions(tc.customOptions)
26+
27+
require.ErrorIs(t, validationResult, tc.expectedResult)
28+
}
29+
}
30+
31+
func TestDetectConfigChanges(t *testing.T) {
32+
boolTrue := true
33+
boolFalse := false
34+
port1 := uint(6000)
35+
port2 := uint(6100)
36+
37+
testCases := []struct {
38+
name string
39+
oldProj *models.ConfigProjection
40+
newProj *models.ConfigProjection
41+
expectedChangedSettings int
42+
expectedRestartSettings int
1343
}{
1444
{
15-
customOptions: []interface{}{"--verbose"},
16-
expectedResult: nil,
45+
name: "no changes",
46+
oldProj: &models.ConfigProjection{Debug: &boolTrue},
47+
newProj: &models.ConfigProjection{Debug: &boolTrue},
48+
expectedChangedSettings: 0,
49+
expectedRestartSettings: 0,
1750
},
1851
{
19-
customOptions: []interface{}{"--exclude-scheme=test_scheme"},
20-
expectedResult: nil,
52+
name: "debug changed",
53+
oldProj: &models.ConfigProjection{Debug: &boolTrue},
54+
newProj: &models.ConfigProjection{Debug: &boolFalse},
55+
expectedChangedSettings: 1,
56+
expectedRestartSettings: 0,
2157
},
2258
{
23-
customOptions: []interface{}{`--exclude-scheme="test_scheme"`},
24-
expectedResult: nil,
59+
name: "port pool changed - requires restart",
60+
oldProj: &models.ConfigProjection{PortPoolFrom: &port1},
61+
newProj: &models.ConfigProjection{PortPoolFrom: &port2},
62+
expectedChangedSettings: 1,
63+
expectedRestartSettings: 1,
2564
},
2665
{
27-
customOptions: []interface{}{"--table=$(echo 'test')"},
28-
expectedResult: errInvalidOption,
66+
name: "multiple changes with restart",
67+
oldProj: &models.ConfigProjection{Debug: &boolTrue, PortPoolFrom: &port1},
68+
newProj: &models.ConfigProjection{Debug: &boolFalse, PortPoolFrom: &port2},
69+
expectedChangedSettings: 2,
70+
expectedRestartSettings: 1,
2971
},
72+
}
73+
74+
for _, tc := range testCases {
75+
t.Run(tc.name, func(t *testing.T) {
76+
changedSettings, restartSettings := detectConfigChanges(tc.oldProj, tc.newProj)
77+
require.Equal(t, tc.expectedChangedSettings, len(changedSettings))
78+
require.Equal(t, tc.expectedRestartSettings, len(restartSettings))
79+
})
80+
}
81+
}
82+
83+
func TestGenerateRestartWarnings(t *testing.T) {
84+
testCases := []struct {
85+
name string
86+
restartSettings []string
87+
expectedWarnings int
88+
}{
89+
{name: "no restart settings", restartSettings: []string{}, expectedWarnings: 0},
90+
{name: "one restart setting", restartSettings: []string{"server.port"}, expectedWarnings: 1},
91+
{name: "multiple restart settings", restartSettings: []string{"server.port", "provision.portPool.from"}, expectedWarnings: 2},
92+
}
93+
94+
for _, tc := range testCases {
95+
t.Run(tc.name, func(t *testing.T) {
96+
warnings := generateRestartWarnings(tc.restartSettings)
97+
require.Equal(t, tc.expectedWarnings, len(warnings))
98+
for _, warning := range warnings {
99+
require.Equal(t, "restart", warning.Type)
100+
require.NotEmpty(t, warning.Message)
101+
}
102+
})
103+
}
104+
}
105+
106+
func TestValidatePortPoolSettings(t *testing.T) {
107+
validFrom := uint(6000)
108+
validTo := uint(6100)
109+
invalidFrom := uint(0)
110+
invalidTo := uint(5000)
111+
112+
testCases := []struct {
113+
name string
114+
proj *models.ConfigProjection
115+
expectErr bool
116+
}{
117+
{name: "valid port pool", proj: &models.ConfigProjection{PortPoolFrom: &validFrom, PortPoolTo: &validTo}, expectErr: false},
118+
{name: "nil port pool", proj: &models.ConfigProjection{}, expectErr: false},
119+
{name: "invalid from port", proj: &models.ConfigProjection{PortPoolFrom: &invalidFrom, PortPoolTo: &validTo}, expectErr: true},
120+
{name: "from greater than to", proj: &models.ConfigProjection{PortPoolFrom: &validTo, PortPoolTo: &invalidTo}, expectErr: true},
121+
}
122+
123+
for _, tc := range testCases {
124+
t.Run(tc.name, func(t *testing.T) {
125+
err := validatePortPoolSettings(tc.proj)
126+
if tc.expectErr {
127+
require.Error(t, err)
128+
} else {
129+
require.NoError(t, err)
130+
}
131+
})
132+
}
133+
}
134+
135+
func TestValidateDiagnosticSettings(t *testing.T) {
136+
validDays := 30
137+
negativeDays := -1
138+
139+
testCases := []struct {
140+
name string
141+
proj *models.ConfigProjection
142+
expectErr bool
143+
}{
144+
{name: "valid retention days", proj: &models.ConfigProjection{LogsRetentionDays: &validDays}, expectErr: false},
145+
{name: "nil retention days", proj: &models.ConfigProjection{}, expectErr: false},
146+
{name: "negative retention days", proj: &models.ConfigProjection{LogsRetentionDays: &negativeDays}, expectErr: true},
147+
}
148+
149+
for _, tc := range testCases {
150+
t.Run(tc.name, func(t *testing.T) {
151+
err := validateDiagnosticSettings(tc.proj)
152+
if tc.expectErr {
153+
require.Error(t, err)
154+
} else {
155+
require.NoError(t, err)
156+
}
157+
})
158+
}
159+
}
160+
161+
func TestValidateEmbeddedUISettings(t *testing.T) {
162+
validPort := 2345
163+
invalidPortLow := 0
164+
invalidPortHigh := 70000
165+
166+
testCases := []struct {
167+
name string
168+
proj *models.ConfigProjection
169+
expectErr bool
170+
}{
171+
{name: "valid port", proj: &models.ConfigProjection{EmbeddedUIPort: &validPort}, expectErr: false},
172+
{name: "nil port", proj: &models.ConfigProjection{}, expectErr: false},
173+
{name: "port too low", proj: &models.ConfigProjection{EmbeddedUIPort: &invalidPortLow}, expectErr: true},
174+
{name: "port too high", proj: &models.ConfigProjection{EmbeddedUIPort: &invalidPortHigh}, expectErr: true},
175+
}
176+
177+
for _, tc := range testCases {
178+
t.Run(tc.name, func(t *testing.T) {
179+
err := validateEmbeddedUISettings(tc.proj)
180+
if tc.expectErr {
181+
require.Error(t, err)
182+
} else {
183+
require.NoError(t, err)
184+
}
185+
})
186+
}
187+
}
188+
189+
func TestValidateWebhookSettings(t *testing.T) {
190+
testCases := []struct {
191+
name string
192+
proj *models.ConfigProjection
193+
expectErr bool
194+
}{
195+
{name: "no webhooks", proj: &models.ConfigProjection{}, expectErr: false},
30196
{
31-
customOptions: []interface{}{"--table=test&table"},
32-
expectedResult: errInvalidOption,
197+
name: "valid webhook",
198+
proj: &models.ConfigProjection{
199+
WebhooksHooks: []models.WebhookHookProjection{{URL: "http://example.com", Trigger: []string{"clone.created"}}},
200+
},
201+
expectErr: false,
33202
},
34203
{
35-
customOptions: []interface{}{5},
36-
expectedResult: errInvalidOptionType,
204+
name: "empty url",
205+
proj: &models.ConfigProjection{
206+
WebhooksHooks: []models.WebhookHookProjection{{URL: "", Trigger: []string{"clone.created"}}},
207+
},
208+
expectErr: true,
209+
},
210+
{
211+
name: "empty trigger",
212+
proj: &models.ConfigProjection{
213+
WebhooksHooks: []models.WebhookHookProjection{{URL: "http://example.com", Trigger: []string{}}},
214+
},
215+
expectErr: true,
216+
},
217+
{
218+
name: "invalid trigger",
219+
proj: &models.ConfigProjection{
220+
WebhooksHooks: []models.WebhookHookProjection{{URL: "http://example.com", Trigger: []string{"invalid.trigger"}}},
221+
},
222+
expectErr: true,
37223
},
38224
}
39225

40226
for _, tc := range testCases {
41-
validationResult := validateCustomOptions(tc.customOptions)
227+
t.Run(tc.name, func(t *testing.T) {
228+
err := validateWebhookSettings(tc.proj)
229+
if tc.expectErr {
230+
require.Error(t, err)
231+
} else {
232+
require.NoError(t, err)
233+
}
234+
})
235+
}
236+
}
42237

43-
require.ErrorIs(t, validationResult, tc.expectedResult)
238+
func TestHasRetrievalSettings(t *testing.T) {
239+
testCases := []struct {
240+
name string
241+
objMap map[string]interface{}
242+
expected bool
243+
}{
244+
{name: "empty map", objMap: map[string]interface{}{}, expected: false},
245+
{name: "has debug only", objMap: map[string]interface{}{"debug": true}, expected: false},
246+
{name: "has host", objMap: map[string]interface{}{"host": "localhost"}, expected: true},
247+
{name: "has timetable", objMap: map[string]interface{}{"timetable": "0 0 * * *"}, expected: true},
248+
{name: "has databases", objMap: map[string]interface{}{"databases": []string{"db1"}}, expected: true},
249+
}
250+
251+
for _, tc := range testCases {
252+
t.Run(tc.name, func(t *testing.T) {
253+
result := hasRetrievalSettings(tc.objMap)
254+
require.Equal(t, tc.expected, result)
255+
})
44256
}
45257
}

‎engine/pkg/models/configuration.go‎

Lines changed: 86 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,34 @@ type ConnectionTest struct {
1212

1313
// ConfigProjection is a projection of the configuration.
1414
type ConfigProjection struct {
15-
Debug *bool `proj:"global.debug"`
16-
DatabaseConfigs map[string]interface{} `proj:"databaseConfigs.configs"`
17-
DockerImage *string `proj:"databaseContainer.dockerImage"`
15+
// Global settings
16+
Debug *bool `proj:"global.debug"`
17+
GlobalDBUsername *string `proj:"global.database.username"`
18+
GlobalDBName *string `proj:"global.database.dbname"`
19+
20+
// Server settings
21+
ServerHost *string `proj:"server.host"`
22+
ServerPort *uint `proj:"server.port" restart:"true"`
23+
24+
// Provision settings
25+
PortPoolFrom *uint `proj:"provision.portPool.from" restart:"true"`
26+
PortPoolTo *uint `proj:"provision.portPool.to" restart:"true"`
27+
UseSudo *bool `proj:"provision.useSudo"`
28+
KeepUserPasswords *bool `proj:"provision.keepUserPasswords"`
29+
CloneAccessAddresses *string `proj:"provision.cloneAccessAddresses"`
30+
ContainerConfig map[string]string `proj:"provision.containerConfig"`
31+
32+
// Database container settings
33+
DockerImage *string `proj:"databaseContainer.dockerImage"`
34+
DatabaseConfigs map[string]interface{} `proj:"databaseConfigs.configs"`
35+
36+
// Cloning settings
37+
MaxIdleMinutes *uint `proj:"cloning.maxIdleMinutes"`
38+
AccessHost *string `proj:"cloning.accessHost"`
39+
40+
// Retrieval settings
1841
Timetable *string `proj:"retrieval.refresh.timetable"`
42+
SkipStartRefresh *bool `proj:"retrieval.refresh.skipStartRefresh"`
1943
DBName *string `proj:"retrieval.spec.logicalDump.options.source.connection.dbname"`
2044
Host *string `proj:"retrieval.spec.logicalDump.options.source.connection.host"`
2145
Password *string `proj:"retrieval.spec.logicalDump.options.source.connection.password" groups:"sensitive"`
@@ -28,4 +52,63 @@ type ConfigProjection struct {
2852
RestoreCustomOptions []interface{} `proj:"retrieval.spec.logicalRestore.options.customOptions"`
2953
IgnoreDumpErrors *bool `proj:"retrieval.spec.logicalDump.options.ignoreErrors"`
3054
IgnoreRestoreErrors *bool `proj:"retrieval.spec.logicalRestore.options.ignoreErrors"`
55+
56+
// Pool Manager settings
57+
PoolMountDir *string `proj:"poolManager.mountDir" restart:"true"`
58+
PoolSelectedPool *string `proj:"poolManager.selectedPool"`
59+
60+
// Observer settings
61+
ObserverReplacementRules map[string]string `proj:"observer.replacementRules"`
62+
63+
// Diagnostic settings
64+
LogsRetentionDays *int `proj:"diagnostic.logsRetentionDays"`
65+
66+
// EmbeddedUI settings
67+
EmbeddedUIEnabled *bool `proj:"embeddedUI.enabled" restart:"true"`
68+
EmbeddedUIDockerImage *string `proj:"embeddedUI.dockerImage"`
69+
EmbeddedUIHost *string `proj:"embeddedUI.host"`
70+
EmbeddedUIPort *int `proj:"embeddedUI.port" restart:"true"`
71+
72+
// Platform settings
73+
PlatformURL *string `proj:"platform.url"`
74+
PlatformOrgKey *string `proj:"platform.orgKey" groups:"sensitive"`
75+
PlatformProjectName *string `proj:"platform.projectName"`
76+
PlatformEnablePersonalToken *bool `proj:"platform.enablePersonalTokens"`
77+
PlatformEnableTelemetry *bool `proj:"platform.enableTelemetry"`
78+
79+
// Webhooks settings
80+
WebhooksHooks []WebhookHookProjection `proj:"webhooks.hooks"`
81+
}
82+
83+
// WebhookHookProjection is a projection for webhook hook configuration.
84+
type WebhookHookProjection struct {
85+
URL string `proj:"url" json:"url"`
86+
Secret string `proj:"secret" json:"secret" groups:"sensitive"`
87+
Trigger []string `proj:"trigger" json:"trigger"`
88+
}
89+
90+
// ConfigUpdateResponse represents the response from a config update with warnings.
91+
type ConfigUpdateResponse struct {
92+
Config interface{} `json:"config"`
93+
Warnings []ConfigWarning `json:"warnings,omitempty"`
94+
RequiresRestart bool `json:"requiresRestart"`
95+
ChangedSettings []string `json:"changedSettings,omitempty"`
96+
RestartSettings []string `json:"restartSettings,omitempty"`
97+
}
98+
99+
// ConfigWarning represents a warning message for configuration changes.
100+
type ConfigWarning struct {
101+
Setting string `json:"setting"`
102+
Message string `json:"message"`
103+
Type string `json:"type"` // "restart", "security", "info"
104+
}
105+
106+
// RestartRequiredSettings lists settings that require a restart when changed.
107+
var RestartRequiredSettings = map[string]string{
108+
"server.port": "Changing the server port requires a restart to take effect",
109+
"provision.portPool.from": "Changing the port pool requires a restart to take effect",
110+
"provision.portPool.to": "Changing the port pool requires a restart to take effect",
111+
"poolManager.mountDir": "Changing the mount directory requires a restart to take effect",
112+
"embeddedUI.enabled": "Enabling or disabling the embedded UI requires a restart to take effect",
113+
"embeddedUI.port": "Changing the embedded UI port requires a restart to take effect",
31114
}

0 commit comments

Comments
 (0)