Merge pull request #3294 from Achilleshiel/fix-copy-repofile

Add RepositoryFile2 Option for secondary repository

changelog/unreleased/issue-3293

@@ -0,0 +1,14 @@
+Enhancement: Add `--repository-file2` option to `init` and `copy` command
+
+The `init` and `copy` command can now be used with the `--repository-file2`
+option or the `$RESTIC_REPOSITORY_FILE2` environment variable.
+These to options are in addition to the `--repo2` flag and allow you to read
+the destination repository from a file.
+
+Using both `--repository-file` and `--repo2` options resulted in an error for
+the `copy` or `init` command. The handling of this combination of options has
+been fixed. A workaround for this issue is to only use `--repo` or `-r` and
+`--repo2` for `init` or `copy`.
+
+https://github.com/restic/restic/issues/3293
+https://github.com/restic/restic/pull/3294

cmd/restic/secondary_repo.go

@@ -9,6 +9,7 @@ import (
 
 type secondaryRepoOptions struct {
 	Repo            string
+	RepositoryFile  string
 	password        string
 	PasswordFile    string
 	PasswordCommand string
@@ -17,18 +18,25 @@ type secondaryRepoOptions struct {
 
 func initSecondaryRepoOptions(f *pflag.FlagSet, opts *secondaryRepoOptions, repoPrefix string, repoUsage string) {
 	f.StringVarP(&opts.Repo, "repo2", "", os.Getenv("RESTIC_REPOSITORY2"), repoPrefix+" `repository` "+repoUsage+" (default: $RESTIC_REPOSITORY2)")
+	f.StringVarP(&opts.RepositoryFile, "repository-file2", "", os.Getenv("RESTIC_REPOSITORY_FILE2"), "`file` from which to read the "+repoPrefix+" repository location "+repoUsage+" (default: $RESTIC_REPOSITORY_FILE2)")
 	f.StringVarP(&opts.PasswordFile, "password-file2", "", os.Getenv("RESTIC_PASSWORD_FILE2"), "`file` to read the "+repoPrefix+" repository password from (default: $RESTIC_PASSWORD_FILE2)")
 	f.StringVarP(&opts.KeyHint, "key-hint2", "", os.Getenv("RESTIC_KEY_HINT2"), "key ID of key to try decrypting the "+repoPrefix+" repository first (default: $RESTIC_KEY_HINT2)")
 	f.StringVarP(&opts.PasswordCommand, "password-command2", "", os.Getenv("RESTIC_PASSWORD_COMMAND2"), "shell `command` to obtain the "+repoPrefix+" repository password from (default: $RESTIC_PASSWORD_COMMAND2)")
 }
 
 func fillSecondaryGlobalOpts(opts secondaryRepoOptions, gopts GlobalOptions, repoPrefix string) (GlobalOptions, error) {
-	if opts.Repo == "" {
-		return GlobalOptions{}, errors.Fatal("Please specify a " + repoPrefix + " repository location (--repo2)")
+	if opts.Repo == "" && opts.RepositoryFile == "" {
+		return GlobalOptions{}, errors.Fatal("Please specify a " + repoPrefix + " repository location (--repo2 or --repository-file2)")
 	}
+
+	if opts.Repo != "" && opts.RepositoryFile != "" {
+		return GlobalOptions{}, errors.Fatal("Options --repo2 and --repository-file2 are mutually exclusive, please specify only one")
+	}
+
 	var err error
 	dstGopts := gopts
 	dstGopts.Repo = opts.Repo
+	dstGopts.RepositoryFile = opts.RepositoryFile
 	dstGopts.PasswordFile = opts.PasswordFile
 	dstGopts.PasswordCommand = opts.PasswordCommand
 	dstGopts.KeyHint = opts.KeyHint

cmd/restic/secondary_repo_test.go

@@ -0,0 +1,132 @@
+package main
+
+import (
+	"io/ioutil"
+	"path/filepath"
+	"testing"
+
+	rtest "github.com/restic/restic/internal/test"
+)
+
+//TestFillSecondaryGlobalOpts tests valid and invalid data on fillSecondaryGlobalOpts-function
+func TestFillSecondaryGlobalOpts(t *testing.T) {
+	//secondaryRepoTestCase defines a struct for test cases
+	type secondaryRepoTestCase struct {
+		Opts     secondaryRepoOptions
+		DstGOpts GlobalOptions
+	}
+
+	//validSecondaryRepoTestCases is a list with test cases that must pass
+	var validSecondaryRepoTestCases = []secondaryRepoTestCase{
+		{
+			// Test if Repo and Password are parsed correctly.
+			Opts: secondaryRepoOptions{
+				Repo:     "backupDst",
+				password: "secretDst",
+			},
+			DstGOpts: GlobalOptions{
+				Repo:     "backupDst",
+				password: "secretDst",
+			},
+		},
+		{
+			// Test if RepositoryFile and PasswordFile are parsed correctly.
+			Opts: secondaryRepoOptions{
+				RepositoryFile: "backupDst",
+				PasswordFile:   "passwordFileDst",
+			},
+			DstGOpts: GlobalOptions{
+				RepositoryFile: "backupDst",
+				password:       "secretDst",
+				PasswordFile:   "passwordFileDst",
+			},
+		},
+		{
+			// Test if RepositoryFile and PasswordCommand are parsed correctly.
+			Opts: secondaryRepoOptions{
+				RepositoryFile:  "backupDst",
+				PasswordCommand: "echo secretDst",
+			},
+			DstGOpts: GlobalOptions{
+				RepositoryFile:  "backupDst",
+				password:        "secretDst",
+				PasswordCommand: "echo secretDst",
+			},
+		},
+	}
+
+	//invalidSecondaryRepoTestCases is a list with test cases that must fail
+	var invalidSecondaryRepoTestCases = []secondaryRepoTestCase{
+		{
+			// Test must fail on no repo given.
+			Opts: secondaryRepoOptions{},
+		},
+		{
+			// Test must fail as Repo and RepositoryFile are both given
+			Opts: secondaryRepoOptions{
+				Repo:           "backupDst",
+				RepositoryFile: "backupDst",
+			},
+		},
+		{
+			// Test must fail as PasswordFile and PasswordCommand are both given
+			Opts: secondaryRepoOptions{
+				Repo:            "backupDst",
+				PasswordFile:    "passwordFileDst",
+				PasswordCommand: "notEmpty",
+			},
+		},
+		{
+			// Test must fail as PasswordFile does not exist
+			Opts: secondaryRepoOptions{
+				Repo:         "backupDst",
+				PasswordFile: "NonExistingFile",
+			},
+		},
+		{
+			// Test must fail as PasswordCommand does not exist
+			Opts: secondaryRepoOptions{
+				Repo:            "backupDst",
+				PasswordCommand: "notEmpty",
+			},
+		},
+		{
+			// Test must fail as no password is given.
+			Opts: secondaryRepoOptions{
+				Repo: "backupDst",
+			},
+		},
+	}
+
+	//gOpts defines the Global options used in the secondary repository tests
+	var gOpts = GlobalOptions{
+		Repo:           "backupSrc",
+		RepositoryFile: "backupSrc",
+		password:       "secretSrc",
+		PasswordFile:   "passwordFileSrc",
+	}
+
+	//Create temp dir to create password file.
+	dir, cleanup := rtest.TempDir(t)
+	defer cleanup()
+
+	cleanup = rtest.Chdir(t, dir)
+	defer cleanup()
+
+	//Create temporary password file
+	err := ioutil.WriteFile(filepath.Join(dir, "passwordFileDst"), []byte("secretDst"), 0666)
+	rtest.OK(t, err)
+
+	// Test all valid cases
+	for _, testCase := range validSecondaryRepoTestCases {
+		DstGOpts, err := fillSecondaryGlobalOpts(testCase.Opts, gOpts, "destination")
+		rtest.OK(t, err)
+		rtest.Equals(t, DstGOpts, testCase.DstGOpts)
+	}
+
+	// Test all invalid cases
+	for _, testCase := range invalidSecondaryRepoTestCases {
+		_, err := fillSecondaryGlobalOpts(testCase.Opts, gOpts, "destination")
+		rtest.Assert(t, err != nil, "Expected error, but function did not return an error")
+	}
+}

doc/045_working_with_repos.rst

@@ -117,8 +117,12 @@ be skipped by later copy runs.
     both the source and destination repository, *may occupy up to twice their
     space* in the destination repository. See below for how to avoid this.
 
-For the destination repository ``--repo2`` the password can be read from
-a file ``--password-file2`` or from a command ``--password-command2``.
+The destination repository is specified with ``--repo2`` or can be read 
+from a file specified via ``--repository-file2``. Both of these options
+can also set as environment variables ``$RESTIC_REPOSITORY2`` or
+``$RESTIC_REPOSITORY_FILE2`` respectively. For the destination repository 
+the password can be read from a file ``--password-file2`` or from a command 
+``--password-command2``.
 Alternatively the environment variables ``$RESTIC_PASSWORD_COMMAND2`` and
 ``$RESTIC_PASSWORD_FILE2`` can be used. It is also possible to directly
 pass the password via ``$RESTIC_PASSWORD2``. The key which should be used