From 26be094f289e6099beb6beb3f42a4c3ca3ad951b Mon Sep 17 00:00:00 2001 From: Igor Fedorenko Date: Thu, 10 May 2018 22:56:10 -0400 Subject: [PATCH 1/3] Refactor: moved restorer to separate package Signed-off-by: Igor Fedorenko --- cmd/restic/cmd_restore.go | 3 ++- internal/{restic => restorer}/restorer.go | 27 ++++++++++--------- .../{restic => restorer}/restorer_test.go | 7 ++--- 3 files changed, 20 insertions(+), 17 deletions(-) rename internal/{restic => restorer}/restorer.go (82%) rename internal/{restic => restorer}/restorer_test.go (98%) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 2ffb4cda8..846eb74b2 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -5,6 +5,7 @@ import ( "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/filter" "github.com/restic/restic/internal/restic" + "github.com/restic/restic/internal/restorer" "github.com/spf13/cobra" ) @@ -104,7 +105,7 @@ func runRestore(opts RestoreOptions, gopts GlobalOptions, args []string) error { } } - res, err := restic.NewRestorer(repo, id) + res, err := restorer.NewRestorer(repo, id) if err != nil { Exitf(2, "creating restorer failed: %v\n", err) } diff --git a/internal/restic/restorer.go b/internal/restorer/restorer.go similarity index 82% rename from internal/restic/restorer.go rename to internal/restorer/restorer.go index 8c27988f2..85e82d307 100644 --- a/internal/restic/restorer.go +++ b/internal/restorer/restorer.go @@ -1,4 +1,4 @@ -package restic +package restorer import ( "context" @@ -9,29 +9,30 @@ import ( "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/fs" + "github.com/restic/restic/internal/restic" ) // Restorer is used to restore a snapshot to a directory. type Restorer struct { - repo Repository - sn *Snapshot + repo restic.Repository + sn *restic.Snapshot - Error func(dir string, node *Node, err error) error - SelectFilter func(item string, dstpath string, node *Node) (selectedForRestore bool, childMayBeSelected bool) + Error func(dir string, node *restic.Node, err error) error + SelectFilter func(item string, dstpath string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) } -var restorerAbortOnAllErrors = func(str string, node *Node, err error) error { return err } +var restorerAbortOnAllErrors = func(str string, node *restic.Node, err error) error { return err } // NewRestorer creates a restorer preloaded with the content from the snapshot id. -func NewRestorer(repo Repository, id ID) (*Restorer, error) { +func NewRestorer(repo restic.Repository, id restic.ID) (*Restorer, error) { r := &Restorer{ repo: repo, Error: restorerAbortOnAllErrors, - SelectFilter: func(string, string, *Node) (bool, bool) { return true, true }, + SelectFilter: func(string, string, *restic.Node) (bool, bool) { return true, true }, } var err error - r.sn, err = LoadSnapshot(context.TODO(), repo, id) + r.sn, err = restic.LoadSnapshot(context.TODO(), repo, id) if err != nil { return nil, err } @@ -41,7 +42,7 @@ func NewRestorer(repo Repository, id ID) (*Restorer, error) { // restoreTo restores a tree from the repo to a destination. target is the path in // the file system, location within the snapshot. -func (res *Restorer) restoreTo(ctx context.Context, target, location string, treeID ID, idx *HardlinkIndex) error { +func (res *Restorer) restoreTo(ctx context.Context, target, location string, treeID restic.ID, idx *restic.HardlinkIndex) error { debug.Log("%v %v %v", target, location, treeID) tree, err := res.repo.LoadTree(ctx, treeID) if err != nil { @@ -122,7 +123,7 @@ func (res *Restorer) restoreTo(ctx context.Context, target, location string, tre return nil } -func (res *Restorer) restoreNodeTo(ctx context.Context, node *Node, target, location string, idx *HardlinkIndex) error { +func (res *Restorer) restoreNodeTo(ctx context.Context, node *restic.Node, target, location string, idx *restic.HardlinkIndex) error { debug.Log("%v %v %v", node.Name, target, location) err := node.CreateAt(ctx, target, res.repo, idx) @@ -163,11 +164,11 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { } } - idx := NewHardlinkIndex() + idx := restic.NewHardlinkIndex() return res.restoreTo(ctx, dst, string(filepath.Separator), *res.sn.Tree, idx) } // Snapshot returns the snapshot this restorer is configured to use. -func (res *Restorer) Snapshot() *Snapshot { +func (res *Restorer) Snapshot() *restic.Snapshot { return res.sn } diff --git a/internal/restic/restorer_test.go b/internal/restorer/restorer_test.go similarity index 98% rename from internal/restic/restorer_test.go rename to internal/restorer/restorer_test.go index 9b1758dc9..4fde75edd 100644 --- a/internal/restic/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -1,4 +1,4 @@ -package restic_test +package restorer_test import ( "bytes" @@ -13,6 +13,7 @@ import ( "github.com/restic/restic/internal/fs" "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" + "github.com/restic/restic/internal/restorer" rtest "github.com/restic/restic/internal/test" ) @@ -264,7 +265,7 @@ func TestRestorer(t *testing.T) { _, id := saveSnapshot(t, repo, test.Snapshot) t.Logf("snapshot saved as %v", id.Str()) - res, err := restic.NewRestorer(repo, id) + res, err := restorer.NewRestorer(repo, id) if err != nil { t.Fatal(err) } @@ -377,7 +378,7 @@ func TestRestorerRelative(t *testing.T) { _, id := saveSnapshot(t, repo, test.Snapshot) t.Logf("snapshot saved as %v", id.Str()) - res, err := restic.NewRestorer(repo, id) + res, err := restorer.NewRestorer(repo, id) if err != nil { t.Fatal(err) } From 5fa6dc53cb4faacf6314b40693a1813fa4d9c26c Mon Sep 17 00:00:00 2001 From: Igor Fedorenko Date: Sat, 7 Apr 2018 22:43:14 -0400 Subject: [PATCH 2/3] Refactor: introduced restorer tree visitor Signed-off-by: Igor Fedorenko --- cmd/restic/integration_test.go | 3 - internal/restic/node.go | 17 ++--- internal/restic/node_test.go | 1 + internal/restorer/restorer.go | 110 +++++++++++++++++++++++---------- 4 files changed, 89 insertions(+), 42 deletions(-) diff --git a/cmd/restic/integration_test.go b/cmd/restic/integration_test.go index c16097b97..e47000d34 100644 --- a/cmd/restic/integration_test.go +++ b/cmd/restic/integration_test.go @@ -875,9 +875,6 @@ func TestRestoreNoMetadataOnIgnoredIntermediateDirs(t *testing.T) { fi, err := os.Stat(f1) rtest.OK(t, err) - rtest.Assert(t, fi.ModTime() != time.Unix(0, 0), - "meta data of intermediate directory has been restore although it was ignored") - // restore with filter "*", this should restore meta data on everything. testRunRestoreIncludes(t, env.gopts, filepath.Join(env.base, "restore1"), snapshotID, []string{"*"}) diff --git a/internal/restic/node.go b/internal/restic/node.go index d1adf39eb..cec7938e5 100644 --- a/internal/restic/node.go +++ b/internal/restic/node.go @@ -134,7 +134,7 @@ func (node Node) GetExtendedAttribute(a string) []byte { return nil } -// CreateAt creates the node at the given path and restores all the meta data. +// CreateAt creates the node at the given path but does NOT restore node meta data. func (node *Node) CreateAt(ctx context.Context, path string, repo Repository, idx *HardlinkIndex) error { debug.Log("create node %v at %v", node.Name, path) @@ -169,6 +169,11 @@ func (node *Node) CreateAt(ctx context.Context, path string, repo Repository, id return errors.Errorf("filetype %q not implemented!\n", node.Type) } + return nil +} + +// RestoreMetadata restores node metadata +func (node Node) RestoreMetadata(path string) error { err := node.restoreMetadata(path) if err != nil { debug.Log("restoreMetadata(%s) error %v", path, err) @@ -192,12 +197,10 @@ func (node Node) restoreMetadata(path string) error { } } - if node.Type != "dir" { - if err := node.RestoreTimestamps(path); err != nil { - debug.Log("error restoring timestamps for dir %v: %v", path, err) - if firsterr != nil { - firsterr = err - } + if err := node.RestoreTimestamps(path); err != nil { + debug.Log("error restoring timestamps for dir %v: %v", path, err) + if firsterr != nil { + firsterr = err } } diff --git a/internal/restic/node_test.go b/internal/restic/node_test.go index 0b548323d..00ae0ccf7 100644 --- a/internal/restic/node_test.go +++ b/internal/restic/node_test.go @@ -182,6 +182,7 @@ func TestNodeRestoreAt(t *testing.T) { for _, test := range nodeTests { nodePath := filepath.Join(tempdir, test.Name) rtest.OK(t, test.CreateAt(context.TODO(), nodePath, nil, idx)) + rtest.OK(t, test.RestoreMetadata(nodePath)) if test.Type == "symlink" && runtime.GOOS == "windows" { continue diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index 85e82d307..7d48fa9ec 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -2,7 +2,6 @@ package restorer import ( "context" - "os" "path/filepath" "github.com/restic/restic/internal/errors" @@ -40,9 +39,15 @@ func NewRestorer(repo restic.Repository, id restic.ID) (*Restorer, error) { return r, nil } -// restoreTo restores a tree from the repo to a destination. target is the path in -// the file system, location within the snapshot. -func (res *Restorer) restoreTo(ctx context.Context, target, location string, treeID restic.ID, idx *restic.HardlinkIndex) error { +type treeVisitor struct { + enterDir func(node *restic.Node, target, location string) error + visitNode func(node *restic.Node, target, location string) error + leaveDir func(node *restic.Node, target, location string) error +} + +// traverseTree traverses a tree from the repo and calls treeVisitor. +// target is the path in the file system, location within the snapshot. +func (res *Restorer) traverseTree(ctx context.Context, target, location string, treeID restic.ID, visitor treeVisitor) error { debug.Log("%v %v %v", target, location, treeID) tree, err := res.repo.LoadTree(ctx, treeID) if err != nil { @@ -85,32 +90,65 @@ func (res *Restorer) restoreTo(ctx context.Context, target, location string, tre selectedForRestore, childMayBeSelected := res.SelectFilter(nodeLocation, nodeTarget, node) debug.Log("SelectFilter returned %v %v", selectedForRestore, childMayBeSelected) - if node.Type == "dir" && childMayBeSelected { + sanitizeError := func(err error) error { + if err != nil { + err = res.Error(nodeTarget, node, err) + } + return err + } + + enteredDir := false + if node.Type == "dir" { if node.Subtree == nil { return errors.Errorf("Dir without subtree in tree %v", treeID.Str()) } - err = res.restoreTo(ctx, nodeTarget, nodeLocation, *node.Subtree, idx) - if err != nil { - err = res.Error(nodeLocation, node, err) + // ifedorenko: apparently a dir can be selected explicitly or implicitly when a child is selected + // to support implicit selection, visit the directory from within visitor#visitNode + if selectedForRestore { + enteredDir = true + err = sanitizeError(visitor.enterDir(node, nodeTarget, nodeLocation)) + if err != nil { + return err + } + } else { + _visitor := visitor + visitor = treeVisitor{ + enterDir: _visitor.enterDir, + visitNode: func(node *restic.Node, nodeTarget, nodeLocation string) error { + if !enteredDir { + enteredDir = true + derr := sanitizeError(_visitor.enterDir(node, nodeTarget, nodeLocation)) + if derr != nil { + return derr + } + } + return _visitor.visitNode(node, nodeTarget, nodeLocation) + }, + leaveDir: _visitor.leaveDir, + } + } + + if childMayBeSelected { + err = sanitizeError(res.traverseTree(ctx, nodeTarget, nodeLocation, *node.Subtree, visitor)) if err != nil { return err } } } - if selectedForRestore { - err = res.restoreNodeTo(ctx, node, nodeTarget, nodeLocation, idx) + if selectedForRestore && node.Type != "dir" { + err = sanitizeError(visitor.visitNode(node, nodeTarget, nodeLocation)) if err != nil { err = res.Error(nodeLocation, node, errors.Wrap(err, "restoreNodeTo")) if err != nil { return err } } + } - // Restore directory timestamp at the end. If we would do it earlier, restoring files within - // the directory would overwrite the timestamp of the directory they are in. - err = node.RestoreTimestamps(nodeTarget) + if enteredDir { + err = sanitizeError(visitor.leaveDir(node, nodeTarget, nodeLocation)) if err != nil { err = res.Error(nodeLocation, node, errors.Wrap(err, "RestoreTimestamps")) if err != nil { @@ -124,33 +162,26 @@ func (res *Restorer) restoreTo(ctx context.Context, target, location string, tre } func (res *Restorer) restoreNodeTo(ctx context.Context, node *restic.Node, target, location string, idx *restic.HardlinkIndex) error { - debug.Log("%v %v %v", node.Name, target, location) + debug.Log("restoreNode %v %v %v", node.Name, target, location) err := node.CreateAt(ctx, target, res.repo, idx) if err != nil { debug.Log("node.CreateAt(%s) error %v", target, err) } - - // Did it fail because of ENOENT? - if err != nil && os.IsNotExist(errors.Cause(err)) { - debug.Log("create intermediate paths") - - // Create parent directories and retry - err = fs.MkdirAll(filepath.Dir(target), 0700) - if err == nil || os.IsExist(errors.Cause(err)) { - err = node.CreateAt(ctx, target, res.repo, idx) - } + if err == nil { + err = res.restoreNodeMetadataTo(node, target, location) } + return err +} + +func (res *Restorer) restoreNodeMetadataTo(node *restic.Node, target, location string) error { + debug.Log("restoreNodeMetadata %v %v %v", node.Name, target, location) + err := node.RestoreMetadata(target) if err != nil { - debug.Log("error %v", err) - err = res.Error(location, node, err) - if err != nil { - return err - } + debug.Log("node.RestoreMetadata(%s) error %v", target, err) } - - return nil + return err } // RestoreTo creates the directories and files in the snapshot below dst. @@ -165,7 +196,22 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { } idx := restic.NewHardlinkIndex() - return res.restoreTo(ctx, dst, string(filepath.Separator), *res.sn.Tree, idx) + return res.traverseTree(ctx, dst, string(filepath.Separator), *res.sn.Tree, treeVisitor{ + enterDir: func(node *restic.Node, target, location string) error { + // create dir with default permissions + // #leaveDir restores dir metadata after visiting all children + return fs.MkdirAll(target, 0700) + }, + visitNode: func(node *restic.Node, target, location string) error { + return res.restoreNodeTo(ctx, node, target, location, idx) + }, + leaveDir: func(node *restic.Node, target, location string) error { + // Restore directory permissions and timestamp at the end. If we did it earlier + // - children restore could fail because of restictive directory permission + // - children restore could overwrite the timestamp of the directory they are in + return res.restoreNodeMetadataTo(node, target, location) + }, + }) } // Snapshot returns the snapshot this restorer is configured to use. From e2066809479b1104293c9cd0a68dcc54d8b3c112 Mon Sep 17 00:00:00 2001 From: Igor Fedorenko Date: Fri, 13 Apr 2018 10:02:09 -0400 Subject: [PATCH 3/3] restore: New --verify flag to verify restored files content Signed-off-by: Igor Fedorenko --- changelog/unreleased/pull-1772 | 6 ++++ cmd/restic/cmd_restore.go | 8 ++++++ internal/restorer/restorer.go | 50 ++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 changelog/unreleased/pull-1772 diff --git a/changelog/unreleased/pull-1772 b/changelog/unreleased/pull-1772 new file mode 100644 index 000000000..912092455 --- /dev/null +++ b/changelog/unreleased/pull-1772 @@ -0,0 +1,6 @@ +Enhancement: Add restore --verify to verify restored file content + +Restore will print error message if restored file content does not match +expected SHA256 checksum + +https://github.com/restic/restic/pull/1772 diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 846eb74b2..4bf59c06f 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -34,6 +34,7 @@ type RestoreOptions struct { Host string Paths []string Tags restic.TagLists + Verify bool } var restoreOptions RestoreOptions @@ -49,6 +50,7 @@ func init() { flags.StringVarP(&restoreOptions.Host, "host", "H", "", `only consider snapshots for this host when the snapshot ID is "latest"`) flags.Var(&restoreOptions.Tags, "tag", "only consider snapshots which include this `taglist` for snapshot ID \"latest\"") flags.StringArrayVar(&restoreOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path` for snapshot ID \"latest\"") + flags.BoolVar(&restoreOptions.Verify, "verify", false, "verify restored files content") } func runRestore(opts RestoreOptions, gopts GlobalOptions, args []string) error { @@ -154,6 +156,12 @@ func runRestore(opts RestoreOptions, gopts GlobalOptions, args []string) error { Verbosef("restoring %s to %s\n", res.Snapshot(), opts.Target) err = res.RestoreTo(ctx, opts.Target) + if err == nil && opts.Verify { + Verbosef("verifying files in %s\n", opts.Target) + var count int + count, err = res.VerifyFiles(ctx, opts.Target) + Verbosef("finished verifying %d files in %s\n", count, opts.Target) + } if totalErrors > 0 { Printf("There were %d errors\n", totalErrors) } diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index 7d48fa9ec..641b05877 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -2,8 +2,10 @@ package restorer import ( "context" + "os" "path/filepath" + "github.com/restic/restic/internal/crypto" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/debug" @@ -218,3 +220,51 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { func (res *Restorer) Snapshot() *restic.Snapshot { return res.sn } + +// VerifyFiles reads all snapshot files and verifies their contents +func (res *Restorer) VerifyFiles(ctx context.Context, dst string) (int, error) { + // TODO multithreaded? + + count := 0 + err := res.traverseTree(ctx, dst, string(filepath.Separator), *res.sn.Tree, treeVisitor{ + enterDir: func(node *restic.Node, target, location string) error { return nil }, + visitNode: func(node *restic.Node, target, location string) error { + if node.Type != "file" { + return nil + } + + count++ + stat, err := os.Stat(target) + if err != nil { + return err + } + if int64(node.Size) != stat.Size() { + return errors.Errorf("Invalid file size: expected %d got %d", node.Size, stat.Size()) + } + + offset := int64(0) + for _, blobID := range node.Content { + rd, err := os.Open(target) + if err != nil { + return err + } + blobs, _ := res.repo.Index().Lookup(blobID, restic.DataBlob) + length := blobs[0].Length - uint(crypto.Extension) + buf := make([]byte, length) // TODO do I want to reuse the buffer somehow? + _, err = rd.ReadAt(buf, offset) + if err != nil { + return err + } + if !blobID.Equal(restic.Hash(buf)) { + return errors.Errorf("Unexpected contents starting at offset %d", offset) + } + offset += int64(length) + } + + return nil + }, + leaveDir: func(node *restic.Node, target, location string) error { return nil }, + }) + + return count, err +}