Skip to content

Commit

Permalink
fix: backrest should only initialize repos explicitly added through W…
Browse files Browse the repository at this point in the history
…ebUI
  • Loading branch information
garethgeorge committed Aug 15, 2024
1 parent 500f2ee commit 62a97a3
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 39 deletions.
11 changes: 11 additions & 0 deletions internal/api/backresthandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
16 changes: 4 additions & 12 deletions internal/orchestrator/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
47 changes: 20 additions & 27 deletions internal/orchestrator/repo/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand All @@ -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)
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
}

0 comments on commit 62a97a3

Please sign in to comment.