diff --git a/internal/config/validate.go b/internal/config/validate.go index 6d460661..d3f4ee32 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -13,7 +13,6 @@ import ( "github.com/garethgeorge/backrest/internal/protoutil" "github.com/hashicorp/go-multierror" "go.uber.org/zap" - "google.golang.org/protobuf/proto" ) func ValidateConfig(c *v1.Config) error { @@ -114,6 +113,19 @@ func validateRepo(repo *v1.Repo) error { } } + if repo.ForgetPolicy != nil { + if repo.ForgetPolicy.GetSchedule() != nil { + if e := protoutil.ValidateSchedule(repo.ForgetPolicy.GetSchedule()); e != nil { + err = multierror.Append(err, fmt.Errorf("forget policy schedule: %w", e)) + } + } + if repo.ForgetPolicy.GetRetention() == nil { + err = multierror.Append(err, errors.New("forget policy must specify a retention policy")) + } else if e := protoutil.ValidateRetentionPolicy(repo.ForgetPolicy.GetRetention()); e != nil { + err = multierror.Append(err, fmt.Errorf("forget policy: %w", e)) + } + } + for _, env := range repo.Env { if !strings.Contains(env, "=") { err = multierror.Append(err, fmt.Errorf("invalid env var %s, must take format KEY=VALUE", env)) @@ -155,11 +167,9 @@ func validatePlan(plan *v1.Plan, repos map[string]*v1.Repo) error { err = multierror.Append(err, fmt.Errorf("repo %q not found", plan.Repo)) } - if plan.Retention != nil && plan.Retention.Policy == nil { - err = multierror.Append(err, errors.New("retention policy must be nil or must specify a policy")) - } else if policyTimeBucketed, ok := plan.Retention.GetPolicy().(*v1.RetentionPolicy_PolicyTimeBucketed); ok { - if proto.Equal(policyTimeBucketed.PolicyTimeBucketed, &v1.RetentionPolicy_TimeBucketedCounts{}) { - err = multierror.Append(err, errors.New("time bucketed policy must specify a non-empty bucket")) + if plan.Retention != nil { + if e := protoutil.ValidateRetentionPolicy(plan.Retention); e != nil { + err = multierror.Append(err, fmt.Errorf("retention: %w", e)) } } diff --git a/internal/config/validate_test.go b/internal/config/validate_test.go index 0ad28b9a..eddb8c5b 100644 --- a/internal/config/validate_test.go +++ b/internal/config/validate_test.go @@ -140,6 +140,88 @@ func TestCleanupOrphanedRemoteReposAndPlans(t *testing.T) { } } +func TestValidateRepoForgetPolicy(t *testing.T) { + validGUID := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + baseConfig := func(repo *v1.Repo) *v1.Config { + return &v1.Config{Instance: "test", Repos: []*v1.Repo{repo}} + } + + tests := []struct { + name string + repo *v1.Repo + wantErr bool + }{ + { + name: "no forget policy is valid", + repo: &v1.Repo{Id: "repo1", Uri: "file:///tmp/repo", Guid: validGUID}, + }, + { + name: "valid forget policy", + repo: &v1.Repo{ + Id: "repo1", Uri: "file:///tmp/repo", Guid: validGUID, + ForgetPolicy: &v1.ForgetPolicy{ + Schedule: &v1.Schedule{Schedule: &v1.Schedule_MaxFrequencyDays{MaxFrequencyDays: 1}}, + Retention: &v1.RetentionPolicy{Policy: &v1.RetentionPolicy_PolicyKeepLastN{PolicyKeepLastN: 5}}, + }, + }, + }, + { + name: "forget policy with nil retention", + repo: &v1.Repo{ + Id: "repo1", Uri: "file:///tmp/repo", Guid: validGUID, + ForgetPolicy: &v1.ForgetPolicy{ + Schedule: &v1.Schedule{Schedule: &v1.Schedule_MaxFrequencyDays{MaxFrequencyDays: 1}}, + }, + }, + wantErr: true, + }, + { + name: "forget policy with empty retention", + repo: &v1.Repo{ + Id: "repo1", Uri: "file:///tmp/repo", Guid: validGUID, + ForgetPolicy: &v1.ForgetPolicy{ + Schedule: &v1.Schedule{Schedule: &v1.Schedule_MaxFrequencyDays{MaxFrequencyDays: 1}}, + Retention: &v1.RetentionPolicy{}, + }, + }, + wantErr: true, + }, + { + name: "forget policy with invalid schedule", + repo: &v1.Repo{ + Id: "repo1", Uri: "file:///tmp/repo", Guid: validGUID, + ForgetPolicy: &v1.ForgetPolicy{ + Schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "bad cron"}}, + Retention: &v1.RetentionPolicy{Policy: &v1.RetentionPolicy_PolicyKeepLastN{PolicyKeepLastN: 5}}, + }, + }, + wantErr: true, + }, + { + name: "forget policy with empty time bucketed retention", + repo: &v1.Repo{ + Id: "repo1", Uri: "file:///tmp/repo", Guid: validGUID, + ForgetPolicy: &v1.ForgetPolicy{ + Schedule: &v1.Schedule{Schedule: &v1.Schedule_MaxFrequencyDays{MaxFrequencyDays: 1}}, + Retention: &v1.RetentionPolicy{Policy: &v1.RetentionPolicy_PolicyTimeBucketed{PolicyTimeBucketed: &v1.RetentionPolicy_TimeBucketedCounts{}}}, + }, + }, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := ValidateConfig(baseConfig(tc.repo)) + if tc.wantErr && err == nil { + t.Error("expected error, got nil") + } else if !tc.wantErr && err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} + func sliceEqual(a, b []string) bool { if len(a) != len(b) { return false diff --git a/internal/orchestrator/tasks/taskforget.go b/internal/orchestrator/tasks/taskforget.go index 5b01a514..11243746 100644 --- a/internal/orchestrator/tasks/taskforget.go +++ b/internal/orchestrator/tasks/taskforget.go @@ -173,7 +173,10 @@ func (t *ScheduledForgetTask) shouldSkip(runner TaskRunner, repoProto *v1.Repo) return false // no previous forget, don't skip } - // Check if any backup completed after the last forget + // Check if any backup completed after the last forget. + // Intentionally not scoped by instance ID: in a sync setup the server receives + // backup operations from remote clients. We want forget to run whenever new + // snapshots appear in the repo regardless of which instance created them. _ = runner.QueryOperations(oplog.Query{}. SetRepoGUID(repoProto.GetGuid()). SetReversed(true), func(op *v1.Operation) error { diff --git a/internal/protoutil/conversion.go b/internal/protoutil/conversion.go index 1976ac36..2ba0e8a7 100644 --- a/internal/protoutil/conversion.go +++ b/internal/protoutil/conversion.go @@ -5,6 +5,7 @@ import ( v1 "github.com/garethgeorge/backrest/gen/go/v1" "github.com/garethgeorge/backrest/pkg/restic" + "google.golang.org/protobuf/proto" ) func SnapshotToProto(s *restic.Snapshot) *v1.ResticSnapshot { @@ -109,6 +110,18 @@ func BackupProgressEntryToBackupError(b *restic.BackupProgressEntry) (*v1.Backup }, nil } +func ValidateRetentionPolicy(p *v1.RetentionPolicy) error { + if p.Policy == nil { + return errors.New("retention policy must specify a policy") + } + if policyTimeBucketed, ok := p.GetPolicy().(*v1.RetentionPolicy_PolicyTimeBucketed); ok { + if proto.Equal(policyTimeBucketed.PolicyTimeBucketed, &v1.RetentionPolicy_TimeBucketedCounts{}) { + return errors.New("time bucketed policy must specify a non-empty bucket") + } + } + return nil +} + func RetentionPolicyFromProto(p *v1.RetentionPolicy) *restic.RetentionPolicy { switch p := p.GetPolicy().(type) { case *v1.RetentionPolicy_PolicyKeepAll: