From 2758d76b77ba64372ce6d3ab2f2766a20cedad71 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 7 May 2022 23:26:59 +0200 Subject: [PATCH] copy: replace --repo2 with --from-repo `init` and `copy` use `--repo2` with two different meaning which has proven to be confusing for users. `--from-repo` now consistently marks a source repository from which data is read. `--repo` is now always the target/destination repository. --- changelog/unreleased/pull-3742 | 19 +++++++ cmd/restic/cmd_copy.go | 10 ++-- cmd/restic/cmd_init.go | 4 +- cmd/restic/integration_test.go | 9 ++-- cmd/restic/secondary_repo.go | 89 ++++++++++++++++++++++++------- cmd/restic/secondary_repo_test.go | 58 +++++++++++++++++++- doc/045_working_with_repos.rst | 24 ++++----- 7 files changed, 171 insertions(+), 42 deletions(-) create mode 100644 changelog/unreleased/pull-3742 diff --git a/changelog/unreleased/pull-3742 b/changelog/unreleased/pull-3742 new file mode 100644 index 000000000..b5ed3b621 --- /dev/null +++ b/changelog/unreleased/pull-3742 @@ -0,0 +1,19 @@ +Change: Replace `--repo2` option used by init/copy with `--from-repo` + +The `init` and the `copy` command can read data from another repository. +However, confusingly `--repo2` referred to the repository from which the +`init` command copies parameters, but for the `copy` command `--repo2` +referred to the copy destination. + +We have introduced a new option `--from-repo` which always refers to the +source repository for both commands. The old parameters names have been +deprecated but still work. To create a new repository and copy all snapshots +to it, the commands are now as follows: + +``` +restic -r /srv/restic-repo-copy init --from-repo /srv/restic-repo --copy-chunker-params +restic -r /srv/restic-repo-copy copy --from-repo /srv/restic-repo +``` + +https://github.com/restic/restic/pull/3742 +https://forum.restic.net/t/restic-repository2-confusion/5017 diff --git a/cmd/restic/cmd_copy.go b/cmd/restic/cmd_copy.go index b2c2c00ef..98007c8d2 100644 --- a/cmd/restic/cmd_copy.go +++ b/cmd/restic/cmd_copy.go @@ -50,17 +50,21 @@ func init() { cmdRoot.AddCommand(cmdCopy) f := cmdCopy.Flags() - initSecondaryRepoOptions(f, ©Options.secondaryRepoOptions, "destination", "to copy snapshots to") + initSecondaryRepoOptions(f, ©Options.secondaryRepoOptions, "destination", "to copy snapshots from") f.StringArrayVarP(©Options.Hosts, "host", "H", nil, "only consider snapshots for this `host`, when no snapshot ID is given (can be specified multiple times)") f.Var(©Options.Tags, "tag", "only consider snapshots which include this `taglist`, when no snapshot ID is given") f.StringArrayVar(©Options.Paths, "path", nil, "only consider snapshots which include this (absolute) `path`, when no snapshot ID is given") } func runCopy(opts CopyOptions, gopts GlobalOptions, args []string) error { - dstGopts, err := fillSecondaryGlobalOpts(opts.secondaryRepoOptions, gopts, "destination") + secondaryGopts, isFromRepo, err := fillSecondaryGlobalOpts(opts.secondaryRepoOptions, gopts, "destination") if err != nil { return err } + if isFromRepo { + // swap global options, if the secondary repo was set via from-repo + gopts, secondaryGopts = secondaryGopts, gopts + } ctx, cancel := context.WithCancel(gopts.ctx) defer cancel() @@ -70,7 +74,7 @@ func runCopy(opts CopyOptions, gopts GlobalOptions, args []string) error { return err } - dstRepo, err := OpenRepository(dstGopts) + dstRepo, err := OpenRepository(secondaryGopts) if err != nil { return err } diff --git a/cmd/restic/cmd_init.go b/cmd/restic/cmd_init.go index 8742990f4..4c0392ff1 100644 --- a/cmd/restic/cmd_init.go +++ b/cmd/restic/cmd_init.go @@ -110,7 +110,7 @@ func runInit(opts InitOptions, gopts GlobalOptions, args []string) error { func maybeReadChunkerPolynomial(opts InitOptions, gopts GlobalOptions) (*chunker.Pol, error) { if opts.CopyChunkerParameters { - otherGopts, err := fillSecondaryGlobalOpts(opts.secondaryRepoOptions, gopts, "secondary") + otherGopts, _, err := fillSecondaryGlobalOpts(opts.secondaryRepoOptions, gopts, "secondary") if err != nil { return nil, err } @@ -124,7 +124,7 @@ func maybeReadChunkerPolynomial(opts InitOptions, gopts GlobalOptions) (*chunker return &pol, nil } - if opts.Repo != "" { + if opts.Repo != "" || opts.RepositoryFile != "" || opts.LegacyRepo != "" || opts.LegacyRepositoryFile != "" { return nil, errors.Fatal("Secondary repository must only be specified when copying the chunker parameters") } return nil, nil diff --git a/cmd/restic/integration_test.go b/cmd/restic/integration_test.go index 6f742d2dd..c04a5a2fb 100644 --- a/cmd/restic/integration_test.go +++ b/cmd/restic/integration_test.go @@ -749,14 +749,17 @@ func TestBackupTags(t *testing.T) { } func testRunCopy(t testing.TB, srcGopts GlobalOptions, dstGopts GlobalOptions) { + gopts := srcGopts + gopts.Repo = dstGopts.Repo + gopts.password = dstGopts.password copyOpts := CopyOptions{ secondaryRepoOptions: secondaryRepoOptions{ - Repo: dstGopts.Repo, - password: dstGopts.password, + Repo: srcGopts.Repo, + password: srcGopts.password, }, } - rtest.OK(t, runCopy(copyOpts, srcGopts, nil)) + rtest.OK(t, runCopy(copyOpts, gopts, nil)) } func TestCopy(t *testing.T) { diff --git a/cmd/restic/secondary_repo.go b/cmd/restic/secondary_repo.go index fa42f19b3..de67d380f 100644 --- a/cmd/restic/secondary_repo.go +++ b/cmd/restic/secondary_repo.go @@ -8,49 +8,98 @@ import ( ) type secondaryRepoOptions struct { + password string + // from-repo options Repo string RepositoryFile string - password string PasswordFile string PasswordCommand string KeyHint string + // repo2 options + LegacyRepo string + LegacyRepositoryFile string + LegacyPasswordFile string + LegacyPasswordCommand string + LegacyKeyHint string } 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)") + f.StringVarP(&opts.LegacyRepo, "repo2", "", os.Getenv("RESTIC_REPOSITORY2"), repoPrefix+" `repository` "+repoUsage+" (default: $RESTIC_REPOSITORY2)") + f.StringVarP(&opts.LegacyRepositoryFile, "repository-file2", "", os.Getenv("RESTIC_REPOSITORY_FILE2"), "`file` from which to read the "+repoPrefix+" repository location "+repoUsage+" (default: $RESTIC_REPOSITORY_FILE2)") + f.StringVarP(&opts.LegacyPasswordFile, "password-file2", "", os.Getenv("RESTIC_PASSWORD_FILE2"), "`file` to read the "+repoPrefix+" repository password from (default: $RESTIC_PASSWORD_FILE2)") + f.StringVarP(&opts.LegacyKeyHint, "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.LegacyPasswordCommand, "password-command2", "", os.Getenv("RESTIC_PASSWORD_COMMAND2"), "shell `command` to obtain the "+repoPrefix+" repository password from (default: $RESTIC_PASSWORD_COMMAND2)") + + // hide repo2 options + _ = f.MarkDeprecated("repo2", "use --repo or --from-repo instead") + _ = f.MarkDeprecated("repository-file2", "use --repository-file or --from-repository-file instead") + _ = f.MarkHidden("password-file2") + _ = f.MarkHidden("key-hint2") + _ = f.MarkHidden("password-command2") + + f.StringVarP(&opts.Repo, "from-repo", "", os.Getenv("RESTIC_FROM_REPOSITORY"), "source `repository` "+repoUsage+" (default: $RESTIC_FROM_REPOSITORY)") + f.StringVarP(&opts.RepositoryFile, "from-repository-file", "", os.Getenv("RESTIC_FROM_REPOSITORY_FILE"), "`file` from which to read the source repository location "+repoUsage+" (default: $RESTIC_FROM_REPOSITORY_FILE)") + f.StringVarP(&opts.PasswordFile, "from-password-file", "", os.Getenv("RESTIC_FROM_PASSWORD_FILE2"), "`file` to read the source repository password from (default: $RESTIC_FROM_PASSWORD_FILE)") + f.StringVarP(&opts.KeyHint, "from-key-hint", "", os.Getenv("RESTIC_FROM_KEY_HINT"), "key ID of key to try decrypting the source repository first (default: $RESTIC_FROM_KEY_HINT)") + f.StringVarP(&opts.PasswordCommand, "from-password-command", "", os.Getenv("RESTIC_FROM_PASSWORD_COMMAND"), "shell `command` to obtain the source repository password from (default: $RESTIC_FROM_PASSWORD_COMMAND)") } -func fillSecondaryGlobalOpts(opts secondaryRepoOptions, gopts GlobalOptions, repoPrefix string) (GlobalOptions, error) { - if opts.Repo == "" && opts.RepositoryFile == "" { - return GlobalOptions{}, errors.Fatal("Please specify a " + repoPrefix + " repository location (--repo2 or --repository-file2)") +func fillSecondaryGlobalOpts(opts secondaryRepoOptions, gopts GlobalOptions, repoPrefix string) (GlobalOptions, bool, error) { + if opts.Repo == "" && opts.RepositoryFile == "" && opts.LegacyRepo == "" && opts.LegacyRepositoryFile == "" { + return GlobalOptions{}, false, errors.Fatal("Please specify a source repository location (--from-repo or --from-repository-file)") } - if opts.Repo != "" && opts.RepositoryFile != "" { - return GlobalOptions{}, errors.Fatal("Options --repo2 and --repository-file2 are mutually exclusive, please specify only one") + hasFromRepo := opts.Repo != "" || opts.RepositoryFile != "" || opts.PasswordFile != "" || + opts.KeyHint != "" || opts.PasswordCommand != "" + hasRepo2 := opts.LegacyRepo != "" || opts.LegacyRepositoryFile != "" || opts.LegacyPasswordFile != "" || + opts.LegacyKeyHint != "" || opts.LegacyPasswordCommand != "" + + if hasFromRepo && hasRepo2 { + return GlobalOptions{}, false, errors.Fatal("Option groups repo2 and from-repo 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 + var pwdEnv string + + if hasFromRepo { + if opts.Repo != "" && opts.RepositoryFile != "" { + return GlobalOptions{}, false, errors.Fatal("Options --from-repo and --from-repository-file are mutually exclusive, please specify only one") + } + + dstGopts.Repo = opts.Repo + dstGopts.RepositoryFile = opts.RepositoryFile + dstGopts.PasswordFile = opts.PasswordFile + dstGopts.PasswordCommand = opts.PasswordCommand + dstGopts.KeyHint = opts.KeyHint + + pwdEnv = "RESTIC_FROM_PASSWORD" + repoPrefix = "source" + } else { + if opts.LegacyRepo != "" && opts.LegacyRepositoryFile != "" { + return GlobalOptions{}, false, errors.Fatal("Options --repo2 and --repository-file2 are mutually exclusive, please specify only one") + } + + dstGopts.Repo = opts.LegacyRepo + dstGopts.RepositoryFile = opts.LegacyRepositoryFile + dstGopts.PasswordFile = opts.LegacyPasswordFile + dstGopts.PasswordCommand = opts.LegacyPasswordCommand + dstGopts.KeyHint = opts.LegacyKeyHint + + pwdEnv = "RESTIC_PASSWORD2" + } + if opts.password != "" { dstGopts.password = opts.password } else { - dstGopts.password, err = resolvePassword(dstGopts, "RESTIC_PASSWORD2") + dstGopts.password, err = resolvePassword(dstGopts, pwdEnv) if err != nil { - return GlobalOptions{}, err + return GlobalOptions{}, false, err } } dstGopts.password, err = ReadPassword(dstGopts, "enter password for "+repoPrefix+" repository: ") if err != nil { - return GlobalOptions{}, err + return GlobalOptions{}, false, err } - return dstGopts, nil + return dstGopts, hasFromRepo, nil } diff --git a/cmd/restic/secondary_repo_test.go b/cmd/restic/secondary_repo_test.go index 47c1816af..0c657ae69 100644 --- a/cmd/restic/secondary_repo_test.go +++ b/cmd/restic/secondary_repo_test.go @@ -14,6 +14,7 @@ func TestFillSecondaryGlobalOpts(t *testing.T) { type secondaryRepoTestCase struct { Opts secondaryRepoOptions DstGOpts GlobalOptions + FromRepo bool } //validSecondaryRepoTestCases is a list with test cases that must pass @@ -28,6 +29,7 @@ func TestFillSecondaryGlobalOpts(t *testing.T) { Repo: "backupDst", password: "secretDst", }, + FromRepo: true, }, { // Test if RepositoryFile and PasswordFile are parsed correctly. @@ -40,6 +42,7 @@ func TestFillSecondaryGlobalOpts(t *testing.T) { password: "secretDst", PasswordFile: "passwordFileDst", }, + FromRepo: true, }, { // Test if RepositoryFile and PasswordCommand are parsed correctly. @@ -52,6 +55,42 @@ func TestFillSecondaryGlobalOpts(t *testing.T) { password: "secretDst", PasswordCommand: "echo secretDst", }, + FromRepo: true, + }, + { + // Test if LegacyRepo and Password are parsed correctly. + Opts: secondaryRepoOptions{ + LegacyRepo: "backupDst", + password: "secretDst", + }, + DstGOpts: GlobalOptions{ + Repo: "backupDst", + password: "secretDst", + }, + }, + { + // Test if LegacyRepositoryFile and LegacyPasswordFile are parsed correctly. + Opts: secondaryRepoOptions{ + LegacyRepositoryFile: "backupDst", + LegacyPasswordFile: "passwordFileDst", + }, + DstGOpts: GlobalOptions{ + RepositoryFile: "backupDst", + password: "secretDst", + PasswordFile: "passwordFileDst", + }, + }, + { + // Test if LegacyRepositoryFile and LegacyPasswordCommand are parsed correctly. + Opts: secondaryRepoOptions{ + LegacyRepositoryFile: "backupDst", + LegacyPasswordCommand: "echo secretDst", + }, + DstGOpts: GlobalOptions{ + RepositoryFile: "backupDst", + password: "secretDst", + PasswordCommand: "echo secretDst", + }, }, } @@ -96,6 +135,20 @@ func TestFillSecondaryGlobalOpts(t *testing.T) { Repo: "backupDst", }, }, + { + // Test must fail as current and legacy options are mixed + Opts: secondaryRepoOptions{ + Repo: "backupDst", + LegacyRepo: "backupDst", + }, + }, + { + // Test must fail as current and legacy options are mixed + Opts: secondaryRepoOptions{ + Repo: "backupDst", + LegacyPasswordCommand: "notEmpty", + }, + }, } //gOpts defines the Global options used in the secondary repository tests @@ -119,14 +172,15 @@ func TestFillSecondaryGlobalOpts(t *testing.T) { // Test all valid cases for _, testCase := range validSecondaryRepoTestCases { - DstGOpts, err := fillSecondaryGlobalOpts(testCase.Opts, gOpts, "destination") + DstGOpts, isFromRepo, err := fillSecondaryGlobalOpts(testCase.Opts, gOpts, "destination") rtest.OK(t, err) rtest.Equals(t, DstGOpts, testCase.DstGOpts) + rtest.Equals(t, isFromRepo, testCase.FromRepo) } // Test all invalid cases for _, testCase := range invalidSecondaryRepoTestCases { - _, err := fillSecondaryGlobalOpts(testCase.Opts, gOpts, "destination") + _, _, 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 5fdbfccdc..8f702bc6c 100644 --- a/doc/045_working_with_repos.rst +++ b/doc/045_working_with_repos.rst @@ -90,7 +90,7 @@ example from a local to a remote repository, you can use the ``copy`` command: .. code-block:: console - $ restic -r /srv/restic-repo copy --repo2 /srv/restic-repo-copy + $ restic -r /srv/restic-repo-copy copy --from-repo /srv/restic-repo repository d6504c63 opened successfully, password is correct repository 3dd0878c opened successfully, password is correct @@ -117,17 +117,17 @@ 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. -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 -for decryption can be selected by passing its ID via the flag ``--key-hint2`` -or the environment variable ``$RESTIC_KEY_HINT2``. +The source repository is specified with ``--from-repo`` or can be read +from a file specified via ``--from-repository-file``. Both of these options +can also be set as environment variables ``$RESTIC_FROM_REPOSITORY`` or +``$RESTIC_FROM_REPOSITORY_FILE``, respectively. For the destination repository +the password can be read from a file ``--from-password-file`` or from a command +``--from-password-command``. +Alternatively the environment variables ``$RESTIC_FROM_PASSWORD_COMMAND`` and +``$RESTIC_FROM_PASSWORD_FILE`` can be used. It is also possible to directly +pass the password via ``$RESTIC_FROM_PASSWORD``. The key which should be used +for decryption can be selected by passing its ID via the flag ``--from-key-hint`` +or the environment variable ``$RESTIC_FROM_KEY_HINT``. .. note:: In case the source and destination repository use the same backend, the configuration options and environment variables used to configure the