From 9ccdba9df68296ad5067819a5a558cf25c7244d6 Mon Sep 17 00:00:00 2001 From: Wouter Horlings Date: Sun, 21 Feb 2021 22:43:01 +0100 Subject: [PATCH 1/2] Add repositoryFile2 option The `init` and `copy` commands can now use `--repository-file2` flag and the `$RESTIC_REPOSITORY_FILE2` environment variable. This also fixes the conflict with the `--repository-file` and `--repo2` flag. Tests are added for the initSecondaryGlobalOpts function. This adds a NOK function to the test helper functions. This NOK tests if err is not nil, and otherwise fail the test. With the NOK function a couple of sad paths are tested in the initSecondaryGlobalOpts function. In total the tests checks wether the following are passed correct: - Password - PasswordFile - Repo - RepositoryFile The following situation must return an error to pass the test: - no Repo or RepositoryFile defined - Repo and RepositoryFile defined both --- changelog/unreleased/issue-3293 | 14 ++++ cmd/restic/secondary_repo.go | 12 ++- cmd/restic/secondary_repo_test.go | 132 ++++++++++++++++++++++++++++++ doc/045_working_with_repos.rst | 8 +- 4 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 changelog/unreleased/issue-3293 create mode 100644 cmd/restic/secondary_repo_test.go diff --git a/changelog/unreleased/issue-3293 b/changelog/unreleased/issue-3293 new file mode 100644 index 000000000..1e6e0e0a9 --- /dev/null +++ b/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 diff --git a/cmd/restic/secondary_repo.go b/cmd/restic/secondary_repo.go index 64d4ade13..fa42f19b3 100644 --- a/cmd/restic/secondary_repo.go +++ b/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 diff --git a/cmd/restic/secondary_repo_test.go b/cmd/restic/secondary_repo_test.go new file mode 100644 index 000000000..ee6565e88 --- /dev/null +++ b/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 PasswordFile 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 on no repo given. + Opts: secondaryRepoOptions{ + Repo: "backupDst", + PasswordFile: "passwordFileDst", + PasswordCommand: "notEmpty", + }, + }, + { + // Test must fail on no repo given. + Opts: secondaryRepoOptions{ + Repo: "backupDst", + PasswordFile: "NonExistingFile", + }, + }, + { + // Test must fail on no repo given. + Opts: secondaryRepoOptions{ + Repo: "backupDst", + PasswordCommand: "notEmpty", + }, + }, + { + // Test must fail on no repo 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") + } +} diff --git a/doc/045_working_with_repos.rst b/doc/045_working_with_repos.rst index b28d7d159..75855a5fe 100644 --- a/doc/045_working_with_repos.rst +++ b/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 From da458a55db7184e030e426e75c708442f477b291 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 8 Mar 2021 22:41:01 +0100 Subject: [PATCH 2/2] Cleanup comments in secondary repo test --- cmd/restic/secondary_repo_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/restic/secondary_repo_test.go b/cmd/restic/secondary_repo_test.go index ee6565e88..47c1816af 100644 --- a/cmd/restic/secondary_repo_test.go +++ b/cmd/restic/secondary_repo_test.go @@ -42,7 +42,7 @@ func TestFillSecondaryGlobalOpts(t *testing.T) { }, }, { - // Test if RepositoryFile and PasswordFile are parsed correctly. + // Test if RepositoryFile and PasswordCommand are parsed correctly. Opts: secondaryRepoOptions{ RepositoryFile: "backupDst", PasswordCommand: "echo secretDst", @@ -69,7 +69,7 @@ func TestFillSecondaryGlobalOpts(t *testing.T) { }, }, { - // Test must fail on no repo given. + // Test must fail as PasswordFile and PasswordCommand are both given Opts: secondaryRepoOptions{ Repo: "backupDst", PasswordFile: "passwordFileDst", @@ -77,21 +77,21 @@ func TestFillSecondaryGlobalOpts(t *testing.T) { }, }, { - // Test must fail on no repo given. + // Test must fail as PasswordFile does not exist Opts: secondaryRepoOptions{ Repo: "backupDst", PasswordFile: "NonExistingFile", }, }, { - // Test must fail on no repo given. + // Test must fail as PasswordCommand does not exist Opts: secondaryRepoOptions{ Repo: "backupDst", PasswordCommand: "notEmpty", }, }, { - // Test must fail on no repo given. + // Test must fail as no password is given. Opts: secondaryRepoOptions{ Repo: "backupDst", },