diff --git a/changelog/unreleased/pull-3619 b/changelog/unreleased/pull-3619 new file mode 100644 index 000000000..1806d56b1 --- /dev/null +++ b/changelog/unreleased/pull-3619 @@ -0,0 +1,12 @@ +Bugfix: Avoid choosing parent snapshots newer than time of current snapshot + +`restic backup` (if a `--parent` is not provided) +previously chose the most recent matching snapshot as the parent snapshot. +However, this didn't make sense when the user passed `--time` +to create a snapshot older than the most recent snapshot. + +Instead, `restic backup` now chooses the most recent snapshot +which is not newer than the snapshot-being-created's timestamp, +to avoid any time travel. + +https://github.com/restic/restic/pull/3619 diff --git a/cmd/restic/cmd_backup.go b/cmd/restic/cmd_backup.go index ab9ee41f0..5332284b2 100644 --- a/cmd/restic/cmd_backup.go +++ b/cmd/restic/cmd_backup.go @@ -103,7 +103,7 @@ func init() { cmdRoot.AddCommand(cmdBackup) f := cmdBackup.Flags() - f.StringVar(&backupOptions.Parent, "parent", "", "use this parent `snapshot` (default: last snapshot in the repo that has the same target files/directories)") + f.StringVar(&backupOptions.Parent, "parent", "", "use this parent `snapshot` (default: last snapshot in the repo that has the same target files/directories, and is not newer than the snapshot time)") f.BoolVarP(&backupOptions.Force, "force", "f", false, `force re-reading the target files/directories (overrides the "parent" flag)`) f.StringArrayVarP(&backupOptions.Excludes, "exclude", "e", nil, "exclude a `pattern` (can be specified multiple times)") f.StringArrayVar(&backupOptions.InsensitiveExcludes, "iexclude", nil, "same as --exclude `pattern` but ignores the casing of filenames") @@ -472,7 +472,7 @@ func collectTargets(opts BackupOptions, args []string) (targets []string, err er // parent returns the ID of the parent snapshot. If there is none, nil is // returned. -func findParentSnapshot(ctx context.Context, repo restic.Repository, opts BackupOptions, targets []string) (parentID *restic.ID, err error) { +func findParentSnapshot(ctx context.Context, repo restic.Repository, opts BackupOptions, targets []string, timeStamp time.Time) (parentID *restic.ID, err error) { // Force using a parent if !opts.Force && opts.Parent != "" { id, err := restic.FindSnapshot(ctx, repo, opts.Parent) @@ -485,7 +485,7 @@ func findParentSnapshot(ctx context.Context, repo restic.Repository, opts Backup // Find last snapshot to set it as parent, if not already set if !opts.Force && parentID == nil { - id, err := restic.FindLatestSnapshot(ctx, repo, targets, []restic.TagList{}, []string{opts.Host}) + id, err := restic.FindLatestSnapshot(ctx, repo, targets, []restic.TagList{}, []string{opts.Host}, &timeStamp) if err == nil { parentID = &id } else if err != restic.ErrNoSnapshotFound { @@ -579,7 +579,7 @@ func runBackup(opts BackupOptions, gopts GlobalOptions, term *termstatus.Termina return err } - parentSnapshotID, err := findParentSnapshot(gopts.ctx, repo, opts, targets) + parentSnapshotID, err := findParentSnapshot(gopts.ctx, repo, opts, targets, timeStamp) if err != nil { return err } diff --git a/cmd/restic/cmd_dump.go b/cmd/restic/cmd_dump.go index 4cb96053c..4449af5ce 100644 --- a/cmd/restic/cmd_dump.go +++ b/cmd/restic/cmd_dump.go @@ -152,7 +152,7 @@ func runDump(opts DumpOptions, gopts GlobalOptions, args []string) error { var id restic.ID if snapshotIDString == "latest" { - id, err = restic.FindLatestSnapshot(ctx, repo, opts.Paths, opts.Tags, opts.Hosts) + id, err = restic.FindLatestSnapshot(ctx, repo, opts.Paths, opts.Tags, opts.Hosts, nil) if err != nil { Exitf(1, "latest snapshot for criteria not found: %v Paths:%v Hosts:%v", err, opts.Paths, opts.Hosts) } diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 0ece13e5f..4d5818593 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -118,7 +118,7 @@ func runRestore(opts RestoreOptions, gopts GlobalOptions, args []string) error { var id restic.ID if snapshotIDString == "latest" { - id, err = restic.FindLatestSnapshot(ctx, repo, opts.Paths, opts.Tags, opts.Hosts) + id, err = restic.FindLatestSnapshot(ctx, repo, opts.Paths, opts.Tags, opts.Hosts, nil) if err != nil { Exitf(1, "latest snapshot for criteria not found: %v Paths:%v Hosts:%v", err, opts.Paths, opts.Hosts) } diff --git a/cmd/restic/find.go b/cmd/restic/find.go index 1297420f9..792f1f6a2 100644 --- a/cmd/restic/find.go +++ b/cmd/restic/find.go @@ -23,7 +23,7 @@ func FindFilteredSnapshots(ctx context.Context, repo *repository.Repository, hos for _, s := range snapshotIDs { if s == "latest" { usedFilter = true - id, err = restic.FindLatestSnapshot(ctx, repo, paths, tags, hosts) + id, err = restic.FindLatestSnapshot(ctx, repo, paths, tags, hosts, nil) if err != nil { Warnf("Ignoring %q, no snapshot matched given filter (Paths:%v Tags:%v Hosts:%v)\n", s, paths, tags, hosts) continue diff --git a/internal/restic/snapshot_find.go b/internal/restic/snapshot_find.go index fe9b943de..f85a2020d 100644 --- a/internal/restic/snapshot_find.go +++ b/internal/restic/snapshot_find.go @@ -13,8 +13,8 @@ import ( // ErrNoSnapshotFound is returned when no snapshot for the given criteria could be found. var ErrNoSnapshotFound = errors.New("no snapshot found") -// FindLatestSnapshot finds latest snapshot with optional target/directory, tags and hostname filters. -func FindLatestSnapshot(ctx context.Context, repo Repository, targets []string, tagLists []TagList, hostnames []string) (ID, error) { +// FindLatestSnapshot finds latest snapshot with optional target/directory, tags, hostname, and timestamp filters. +func FindLatestSnapshot(ctx context.Context, repo Repository, targets []string, tagLists []TagList, hostnames []string, timeStamp *time.Time) (ID, error) { var err error absTargets := make([]string, 0, len(targets)) for _, target := range targets { @@ -38,6 +38,10 @@ func FindLatestSnapshot(ctx context.Context, repo Repository, targets []string, return errors.Errorf("Error loading snapshot %v: %v", id.Str(), err) } + if timeStamp != nil && snapshot.Time.After(*timeStamp) { + return nil + } + if snapshot.Time.Before(latest) { return nil } diff --git a/internal/restic/snapshot_find_test.go b/internal/restic/snapshot_find_test.go new file mode 100644 index 000000000..d4fedcc9d --- /dev/null +++ b/internal/restic/snapshot_find_test.go @@ -0,0 +1,47 @@ +package restic_test + +import ( + "context" + "testing" + + "github.com/restic/restic/internal/repository" + "github.com/restic/restic/internal/restic" +) + +func TestFindLatestSnapshot(t *testing.T) { + repo, cleanup := repository.TestRepository(t) + defer cleanup() + + restic.TestCreateSnapshot(t, repo, parseTimeUTC("2015-05-05 05:05:05"), 1, 0) + restic.TestCreateSnapshot(t, repo, parseTimeUTC("2017-07-07 07:07:07"), 1, 0) + latestSnapshot := restic.TestCreateSnapshot(t, repo, parseTimeUTC("2019-09-09 09:09:09"), 1, 0) + + id, err := restic.FindLatestSnapshot(context.TODO(), repo, []string{}, []restic.TagList{}, []string{"foo"}, nil) + if err != nil { + t.Fatalf("FindLatestSnapshot returned error: %v", err) + } + + if id != *latestSnapshot.ID() { + t.Errorf("FindLatestSnapshot returned wrong snapshot ID: %v", id) + } +} + +func TestFindLatestSnapshotWithMaxTimestamp(t *testing.T) { + repo, cleanup := repository.TestRepository(t) + defer cleanup() + + restic.TestCreateSnapshot(t, repo, parseTimeUTC("2015-05-05 05:05:05"), 1, 0) + desiredSnapshot := restic.TestCreateSnapshot(t, repo, parseTimeUTC("2017-07-07 07:07:07"), 1, 0) + restic.TestCreateSnapshot(t, repo, parseTimeUTC("2019-09-09 09:09:09"), 1, 0) + + maxTimestamp := parseTimeUTC("2018-08-08 08:08:08") + + id, err := restic.FindLatestSnapshot(context.TODO(), repo, []string{}, []restic.TagList{}, []string{"foo"}, &maxTimestamp) + if err != nil { + t.Fatalf("FindLatestSnapshot returned error: %v", err) + } + + if id != *desiredSnapshot.ID() { + t.Errorf("FindLatestSnapshot returned wrong snapshot ID: %v", id) + } +}