additional validations

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))
 		}
 	}
 

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

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 {

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: