From 62a97a335df3858a53eba34e7b7c0f69e3875d88 Mon Sep 17 00:00:00 2001 From: garethgeorge Date: Wed, 14 Aug 2024 17:23:59 -0700 Subject: [PATCH] fix: backrest should only initialize repos explicitly added through WebUI --- internal/api/backresthandler_test.go | 11 ++++++ internal/orchestrator/repo/repo.go | 16 +++------ internal/orchestrator/repo/repo_test.go | 47 +++++++++++-------------- 3 files changed, 35 insertions(+), 39 deletions(-) diff --git a/internal/api/backresthandler_test.go b/internal/api/backresthandler_test.go index 970e7a5a..8d9b09ec 100644 --- a/internal/api/backresthandler_test.go +++ b/internal/api/backresthandler_test.go @@ -668,6 +668,17 @@ func createSystemUnderTest(t *testing.T, config config.ConfigStore) systemUnderT t.Fatalf("Failed to create orchestrator: %v", err) } + for _, repo := range cfg.Repos { + rorch, err := orch.GetRepoOrchestrator(repo.Id) + if err != nil { + t.Fatalf("Failed to get repo %s: %v", repo.Id, err) + } + + if err := rorch.Init(context.Background()); err != nil { + t.Fatalf("Failed to init repo %s: %v", repo.Id, err) + } + } + h := NewBackrestHandler(config, orch, oplog, logStore) return systemUnderTest{ diff --git a/internal/orchestrator/repo/repo.go b/internal/orchestrator/repo/repo.go index da00b434..b7dd75de 100644 --- a/internal/orchestrator/repo/repo.go +++ b/internal/orchestrator/repo/repo.go @@ -23,11 +23,10 @@ import ( type RepoOrchestrator struct { mu sync.Mutex - l *zap.Logger - config *v1.Config - repoConfig *v1.Repo - repo *restic.Repo - initialized bool + l *zap.Logger + config *v1.Config + repoConfig *v1.Repo + repo *restic.Repo } // NewRepoOrchestrator accepts a config and a repo that is configured with the properties of that config object. @@ -123,13 +122,6 @@ func (r *RepoOrchestrator) Backup(ctx context.Context, plan *v1.Plan, progressCa r.mu.Lock() defer r.mu.Unlock() - if !r.initialized { - if err := r.repo.Init(ctx); err != nil { - return nil, fmt.Errorf("failed to initialize repo: %w", err) - } - r.initialized = true - } - snapshots, err := r.SnapshotsForPlan(ctx, plan) if err != nil { return nil, fmt.Errorf("failed to get snapshots for plan: %w", err) diff --git a/internal/orchestrator/repo/repo_test.go b/internal/orchestrator/repo/repo_test.go index 9c490ace..ee842a9f 100644 --- a/internal/orchestrator/repo/repo_test.go +++ b/internal/orchestrator/repo/repo_test.go @@ -68,10 +68,7 @@ func TestBackup(t *testing.T) { t.Skip("skipping on windows") } - orchestrator, err := NewRepoOrchestrator(configForTest, tc.repo, helpers.ResticBinary(t)) - if err != nil { - t.Fatalf("failed to create repo orchestrator: %v", err) - } + orchestrator := initRepoHelper(t, configForTest, tc.repo) summary, err := orchestrator.Backup(context.Background(), tc.plan, nil) if err != nil { @@ -116,10 +113,7 @@ func TestSnapshotParenting(t *testing.T) { }, } - orchestrator, err := NewRepoOrchestrator(configForTest, r, helpers.ResticBinary(t)) - if err != nil { - t.Fatalf("failed to create repo orchestrator: %v", err) - } + orchestrator := initRepoHelper(t, configForTest, r) for i := 0; i < 4; i++ { for _, plan := range plans { @@ -181,7 +175,6 @@ func TestEnvVarPropagation(t *testing.T) { t.Parallel() repo := t.TempDir() - testData := test.CreateTestData(t) // create a new repo with cache disabled for testing r := &v1.Repo{ @@ -191,18 +184,12 @@ func TestEnvVarPropagation(t *testing.T) { Env: []string{"RESTIC_PASSWORD=${MY_FOO}"}, } - plan := &v1.Plan{ - Id: "test", - Repo: "test", - Paths: []string{testData}, - } - orchestrator, err := NewRepoOrchestrator(configForTest, r, helpers.ResticBinary(t)) if err != nil { t.Fatalf("failed to create repo orchestrator: %v", err) } - _, err = orchestrator.Backup(context.Background(), plan, nil) + err = orchestrator.Init(context.Background()) if err == nil || !strings.Contains(err.Error(), "password") { t.Fatalf("expected error about RESTIC_PASSWORD, got: %v", err) } @@ -214,14 +201,10 @@ func TestEnvVarPropagation(t *testing.T) { t.Fatalf("failed to create repo orchestrator: %v", err) } - summary, err := orchestrator.Backup(context.Background(), plan, nil) + err = orchestrator.Init(context.Background()) if err != nil { t.Fatalf("backup error: %v", err) } - - if summary.SnapshotId == "" { - t.Fatal("expected snapshot id") - } } func TestCheck(t *testing.T) { @@ -259,14 +242,10 @@ func TestCheck(t *testing.T) { for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - orchestrator, err := NewRepoOrchestrator(configForTest, tc.repo, helpers.ResticBinary(t)) - if err != nil { - t.Fatalf("failed to create repo orchestrator: %v", err) - } - + orchestrator := initRepoHelper(t, configForTest, tc.repo) buf := bytes.NewBuffer(nil) - err = orchestrator.Init(context.Background()) + err := orchestrator.Init(context.Background()) if err != nil { t.Fatalf("init error: %v", err) } @@ -279,3 +258,17 @@ func TestCheck(t *testing.T) { }) } } + +func initRepoHelper(t *testing.T, config *v1.Config, repo *v1.Repo) *RepoOrchestrator { + orchestrator, err := NewRepoOrchestrator(config, repo, helpers.ResticBinary(t)) + if err != nil { + t.Fatalf("failed to create repo orchestrator: %v", err) + } + + err = orchestrator.Init(context.Background()) + if err != nil { + t.Fatalf("init error: %v", err) + } + + return orchestrator +}