Skip to content

Commit b9bd46c

Browse files
korotkov-aerospikekorotkov-aerospike
authored
APPS-1836 Ensure immutable routine configuration in backup and restore (#468)
* remove unused routine name param * add Name to routine object * pass routine instead of name * fix unittests * rename last routine string * remove unused error * routine provider interface * retention manager test * add routineExists * return routine copy in config * copy routine only on backup and restore * merge v3 * delete accidental whitespace * remove redundant exists method * add test for copy * linter * remove unused config parameter
1 parent 11925e0 commit b9bd46c

38 files changed

+625
-537
lines changed

‎build/scripts/generate-mocks.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ generate_mocks() {
3535

3636
generate_mocks \
3737
"pkg/service" \
38-
"RestoreManager,BackupReaderWriter,BackupReader,RunningBackupsRegistry,RetentionManager,ClusterConfigWriter,CancelableBackupHandler,PathService,HistoryManager"
38+
"RestoreManager,BackupReaderWriter,BackupReader,RunningBackupsRegistry,RetentionManager,ClusterConfigWriter,CancelableBackupHandler,PathService,routineProvider,HistoryManager"
3939

4040
generate_mocks \
4141
"pkg/service/restoreexecutor" \

‎cmd/backup/main.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ func initComponents(ctx context.Context, configFile string, remote bool) (
130130
registry := service.NewRunningBackupsRegistry(history, config)
131131

132132
var routineStorage u.LockMap
133-
retentionManager := service.NewBackupRetentionManager(backendService, config, &routineStorage)
134-
clusterConfigWriter := service.NewClusterConfigWriter(clientManager, config, pathService)
133+
retentionManager := service.NewBackupRetentionManager(backendService, &routineStorage)
134+
clusterConfigWriter := service.NewClusterConfigWriter(clientManager, pathService)
135135
backupExecutor := backupexecutor.NewDefaultBackupExecutor()
136136
backupComponents := service.NewBackupComponents(
137137
clientManager, backupExecutor, registry, retentionManager,
@@ -149,7 +149,7 @@ func initComponents(ctx context.Context, configFile string, remote bool) (
149149
restoreMgr := service.NewRestoreManager(
150150
restoreexecutor.NewRestore(), clientManager, restoreJobs, backendService, &routineStorage)
151151

152-
configRetriever := service.NewConfigRetriever(backendService, config, pathService)
152+
configRetriever := service.NewConfigRetriever(backendService, pathService)
153153
httpService := handlers.NewService(
154154
ctx,
155155
config,

‎internal/server/handlers/backup.go‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func (s *Service) GetAllFullBackups(w http.ResponseWriter, r *http.Request) {
3131
return
3232
}
3333

34-
result, err := s.readAllBackups(r.Context(), func(routine string) service.BackupFilter {
34+
result, err := s.readAllBackups(r.Context(), func(routine *model.BackupRoutine) service.BackupFilter {
3535
return service.NewFullBackupFilter(routine).WithTimeBounds(timeBounds)
3636
})
3737
if err != nil {
@@ -61,15 +61,15 @@ func (s *Service) GetFullBackupsForRoutine(w http.ResponseWriter, r *http.Reques
6161
return
6262
}
6363

64-
routine := r.PathValue("name")
65-
if routine == "" {
64+
routineName := r.PathValue("name")
65+
if routineName == "" {
6666
httpError(w, errMissingRoutineName)
6767
return
6868
}
6969

70-
_, found := s.config.Routine(routine)
70+
routine, found := s.config.Routine(routineName)
7171
if !found {
72-
httpError(w, errRoutineNotFound(routine))
72+
httpError(w, errRoutineNotFound(routineName))
7373
return
7474
}
7575

@@ -101,7 +101,7 @@ func (s *Service) GetAllIncrementalBackups(w http.ResponseWriter, r *http.Reques
101101
return
102102
}
103103

104-
result, err := s.readAllBackups(r.Context(), func(routine string) service.BackupFilter {
104+
result, err := s.readAllBackups(r.Context(), func(routine *model.BackupRoutine) service.BackupFilter {
105105
return service.NewIncrementalBackupFilter(routine).WithTimeBounds(timeBounds)
106106
})
107107
if err != nil {
@@ -131,15 +131,15 @@ func (s *Service) GetIncrementalBackupsForRoutine(w http.ResponseWriter, r *http
131131
return
132132
}
133133

134-
routine := r.PathValue("name")
135-
if routine == "" {
134+
routineName := r.PathValue("name")
135+
if routineName == "" {
136136
httpError(w, errMissingRoutineName)
137137
return
138138
}
139139

140-
_, found := s.config.Routine(routine)
140+
routine, found := s.config.Routine(routineName)
141141
if !found {
142-
httpError(w, errRoutineNotFound(routine))
142+
httpError(w, errRoutineNotFound(routineName))
143143
return
144144
}
145145

@@ -155,10 +155,10 @@ func (s *Service) GetIncrementalBackupsForRoutine(w http.ResponseWriter, r *http
155155

156156
func (s *Service) readAllBackups(
157157
ctx context.Context,
158-
filter func(routine string) service.BackupFilter,
158+
filter func(routine *model.BackupRoutine) service.BackupFilter,
159159
) (map[string][]*dto.BackupDetails, error) {
160160
result := make(map[string][]*dto.BackupDetails)
161-
for routine := range s.config.Routines() {
161+
for _, routine := range s.config.Routines() {
162162
routineBackups, err := s.readBackupsForRoutine(ctx, filter(routine))
163163
if err != nil {
164164
if errors.Is(err, service.ErrNotFound) {
@@ -168,7 +168,7 @@ func (s *Service) readAllBackups(
168168
return nil, err
169169
}
170170

171-
result[routine] = routineBackups
171+
result[routine.Name] = routineBackups
172172
}
173173

174174
return result, nil
@@ -262,13 +262,13 @@ func (s *Service) GetCurrentBackupInfo(w http.ResponseWriter, r *http.Request) {
262262
return
263263
}
264264

265-
_, found := s.config.Routine(routineName)
265+
routine, found := s.config.Routine(routineName)
266266
if !found {
267267
httpError(w, errRoutineNotFound(routineName))
268268
return
269269
}
270270

271-
currentBackups := dto.NewRoutineStateFromModel(s.registry.GetRoutineState(routineName))
271+
currentBackups := dto.NewRoutineStateFromModel(s.registry.GetRoutineState(routine))
272272
httpOK(w, currentBackups)
273273
}
274274

@@ -288,8 +288,7 @@ func (s *Service) CancelCurrentBackup(w http.ResponseWriter, r *http.Request) {
288288
return
289289
}
290290

291-
_, found := s.config.Routine(routineName)
292-
if !found {
291+
if _, found := s.config.Routine(routineName); !found {
293292
httpError(w, errRoutineNotFound(routineName))
294293
return
295294
}

‎internal/server/handlers/backup_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestService_GetAllFullBackups(t *testing.T) {
3030
},
3131
setupMock: func(m *MockBackupBackendService) {
3232
m.On("GetBackups", mock.Anything, mock.Anything).
33-
Return([]model.BackupDetails{{Key: "backup1", Routine: "routine1", BackupMetadata: model.BackupMetadata{
33+
Return([]model.BackupDetails{{Key: "backup1", BackupMetadata: model.BackupMetadata{
3434
Created: time.UnixMilli(1000).In(time.UTC),
3535
Finished: time.UnixMilli(5000).In(time.UTC),
3636
}}}, nil)
@@ -63,7 +63,7 @@ func TestService_GetAllFullBackups(t *testing.T) {
6363
t.Run(tt.name, func(t *testing.T) {
6464
mockBackends := &MockBackupBackendService{}
6565
cfg := model.NewConfig()
66-
_ = cfg.AddRoutine("routine1", &model.BackupRoutine{})
66+
_ = cfg.AddRoutine(&model.BackupRoutine{Name: "routine1"})
6767

6868
tt.setupMock(mockBackends)
6969

‎internal/server/handlers/config_routine.go‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package handlers
22

33
import (
4+
"errors"
45
"net/http"
56

67
"github.com/aerospike/aerospike-backup-service/v3/pkg/dto"
@@ -18,8 +19,6 @@ import (
1819
// @Param routine body dto.BackupRoutine true "Backup routine details"
1920
// @Success 201
2021
// @Failure 400 {string} string
21-
//
22-
//nolint:dupl
2322
func (s *Service) AddRoutine(w http.ResponseWriter, r *http.Request) {
2423
name := r.PathValue("name")
2524
if name == "" {
@@ -32,14 +31,14 @@ func (s *Service) AddRoutine(w http.ResponseWriter, r *http.Request) {
3231
return
3332
}
3433

35-
toModel, err := newRoutine.ToModel(s.config.BackupConfigCopy())
34+
toModel, err := newRoutine.ToModel(s.config.BackupConfigCopy(), name)
3635
if err != nil {
3736
httpError(w, errBadRequest(err))
3837
return
3938
}
4039

4140
if err = s.changeConfig(r.Context(), func(config *model.Config) error {
42-
err := config.AddRoutine(name, toModel)
41+
err := config.AddRoutine(toModel)
4342
if err != nil {
4443
return err
4544
}
@@ -87,8 +86,8 @@ func (s *Service) ReadRoutine(w http.ResponseWriter, r *http.Request) {
8786
httpError(w, errMissingRoutineName)
8887
return
8988
}
90-
routine, ok := s.config.Routines()[routineName]
91-
if !ok {
89+
routine, found := s.config.Routine(routineName)
90+
if !found {
9291
httpError(w, errRoutineNotFound(routineName))
9392
return
9493
}
@@ -106,8 +105,6 @@ func (s *Service) ReadRoutine(w http.ResponseWriter, r *http.Request) {
106105
// @Param routine body dto.BackupRoutine true "Backup routine details"
107106
// @Success 200
108107
// @Failure 400 {string} string
109-
//
110-
//nolint:dupl
111108
func (s *Service) UpdateRoutine(w http.ResponseWriter, r *http.Request) {
112109
name := r.PathValue("name")
113110
if name == "" {
@@ -121,7 +118,7 @@ func (s *Service) UpdateRoutine(w http.ResponseWriter, r *http.Request) {
121118
return
122119
}
123120

124-
toModel, err := updatedRoutine.ToModel(s.config.BackupConfigCopy())
121+
toModel, err := updatedRoutine.ToModel(s.config.BackupConfigCopy(), name)
125122
if err != nil {
126123
httpError(w, errBadRequest(err))
127124
return
@@ -184,16 +181,15 @@ func (s *Service) EnableRoutine(w http.ResponseWriter, r *http.Request) {
184181
httpError(w, errMissingRoutineName)
185182
return
186183
}
187-
_, found := s.config.Routine(routineName)
188-
if !found {
189-
httpError(w, errRoutineNotFound(routineName))
190-
return
191-
}
192184

193185
err := s.changeConfig(r.Context(), func(config *model.Config) error {
194186
return config.ToggleRoutineDisabled(routineName, false)
195187
})
196188
if err != nil {
189+
if errors.Is(err, model.ErrNotFound) {
190+
httpError(w, errRoutineNotFound(routineName))
191+
return
192+
}
197193
httpError(w, errBadRequest(err))
198194
return
199195
}
@@ -216,16 +212,15 @@ func (s *Service) DisableRoutine(w http.ResponseWriter, r *http.Request) {
216212
httpError(w, errMissingRoutineName)
217213
return
218214
}
219-
_, found := s.config.Routine(routineName)
220-
if !found {
221-
httpError(w, errRoutineNotFound(routineName))
222-
return
223-
}
224215

225216
err := s.changeConfig(r.Context(), func(config *model.Config) error {
226217
return config.ToggleRoutineDisabled(routineName, true)
227218
})
228219
if err != nil {
220+
if errors.Is(err, model.ErrNotFound) {
221+
httpError(w, errRoutineNotFound(routineName))
222+
return
223+
}
229224
httpError(w, errBadRequest(err))
230225
return
231226
}

‎internal/server/handlers/config_routine_test.go‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ func TestAddRoutine(t *testing.T) {
5858
func TestReadRoutines(t *testing.T) {
5959
svc := setupTestService()
6060
svc.config = model.NewConfig()
61-
_ = svc.config.AddRoutine("routine1", &model.BackupRoutine{})
62-
_ = svc.config.AddRoutine("routine2", &model.BackupRoutine{})
61+
_ = svc.config.AddRoutine(&model.BackupRoutine{Name: "routine1"})
62+
_ = svc.config.AddRoutine(&model.BackupRoutine{Name: "routine2"})
6363

6464
req := httptest.NewRequest(http.MethodGet, "/v1/config/routines", nil)
6565
w := httptest.NewRecorder()
@@ -87,12 +87,11 @@ func TestReadRoutine(t *testing.T) {
8787
{
8888
name: "existing routine",
8989
routineName: "test-routine",
90-
routine: &model.BackupRoutine{},
90+
routine: &model.BackupRoutine{Name: "test-routine"},
9191
expectedStatus: http.StatusOK,
9292
},
9393
{
9494
name: "missing routine name",
95-
routineName: "",
9695
expectedStatus: http.StatusBadRequest,
9796
expectedError: errMissingRoutineName.Error(),
9897
},
@@ -108,7 +107,7 @@ func TestReadRoutine(t *testing.T) {
108107
t.Run(tt.name, func(t *testing.T) {
109108
svc := setupTestService()
110109
if tt.routine != nil {
111-
_ = svc.config.AddRoutine(tt.routineName, tt.routine)
110+
_ = svc.config.AddRoutine(tt.routine)
112111
}
113112

114113
req := httptest.NewRequest(http.MethodGet, "/v1/config/routines/"+tt.routineName, nil)
@@ -167,7 +166,6 @@ func TestUpdateRoutine(t *testing.T) {
167166
}
168167
}
169168

170-
//nolint:dupl
171169
func TestDeleteRoutine(t *testing.T) {
172170
tests := []struct {
173171
name string
@@ -197,7 +195,7 @@ func TestDeleteRoutine(t *testing.T) {
197195
for _, tt := range tests {
198196
t.Run(tt.name, func(t *testing.T) {
199197
svc := setupTestService()
200-
_ = svc.config.AddRoutine("test-routine", &model.BackupRoutine{})
198+
_ = svc.config.AddRoutine(&model.BackupRoutine{Name: "test-routine"})
201199

202200
req := httptest.NewRequest(http.MethodDelete, "/v1/config/routines/"+tt.routineName, nil)
203201
req.SetPathValue("name", tt.routineName)
@@ -219,16 +217,19 @@ func TestEnableRoutine(t *testing.T) {
219217
routineName string
220218
expectedStatus int
221219
expectedError string
220+
addRoutine bool
222221
}{
223222
{
224223
name: "successful enable",
225224
routineName: "test-routine",
225+
addRoutine: true,
226226
expectedStatus: http.StatusNoContent,
227227
},
228228
{
229229
name: "missing routine name",
230230
routineName: "",
231231
expectedStatus: http.StatusBadRequest,
232+
addRoutine: true,
232233
expectedError: errMissingRoutineName.Error(),
233234
},
234235
{
@@ -244,8 +245,12 @@ func TestEnableRoutine(t *testing.T) {
244245
svc := setupTestService()
245246
routine := &model.BackupRoutine{
246247
Disabled: true,
248+
Name: tt.routineName,
249+
}
250+
251+
if tt.addRoutine {
252+
_ = svc.config.AddRoutine(routine)
247253
}
248-
_ = svc.config.AddRoutine("test-routine", routine)
249254

250255
req := httptest.NewRequest(http.MethodPut, "/v1/config/routines/"+tt.routineName+"/enable", nil)
251256
req.SetPathValue("name", tt.routineName)
@@ -297,8 +302,10 @@ func TestDisableRoutine(t *testing.T) {
297302
svc.registry = mockRegistry
298303
mockRegistry.On("Cancel", "test-routine").Once()
299304

300-
routine := &model.BackupRoutine{}
301-
_ = svc.config.AddRoutine("test-routine", routine)
305+
routine := &model.BackupRoutine{
306+
Name: "test-routine",
307+
}
308+
_ = svc.config.AddRoutine(routine)
302309

303310
req := httptest.NewRequest(http.MethodPut, "/v1/config/routines/"+tt.routineName+"/disable", nil)
304311
req.SetPathValue("name", tt.routineName)

0 commit comments

Comments
 (0)