diff --git a/CHANGELOG.md b/CHANGELOG.md index a0c4cc2cc..67446bda5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,10 +19,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Groups and Teams backups no longer fail when a resource has no display name. - Contacts in-place restore failed if the restore destination was empty. - Link shares with external users are now backed up and restored as expected +- Ensure persistent repo config is populated on repo init if repo init failed partway through during the previous init attempt. ### Changed - When running `backup details` on an empty backup returns a more helpful error message. - Backup List additionally shows the data category for each backup. +- Remove hidden `--succeed-if-exists` flag for repo init. Repo init will now succeed without error if run on an existing repo with the same passphrase. ### Known issues - Backing up a group mailbox item may fail if it has a very large number of attachments (500+). diff --git a/src/cli/flags/filesystem.go b/src/cli/flags/filesystem.go index 8ba330d02..be2cf9016 100644 --- a/src/cli/flags/filesystem.go +++ b/src/cli/flags/filesystem.go @@ -28,13 +28,6 @@ func AddFilesystemFlags(cmd *cobra.Command) { "", "path to local or network storage") cobra.CheckErr(cmd.MarkFlagRequired(FilesystemPathFN)) - - fs.BoolVar( - &SucceedIfExistsFV, - SucceedIfExistsFN, - false, - "Exit with success if the repo has already been initialized.") - cobra.CheckErr(fs.MarkHidden("succeed-if-exists")) } func FilesystemFlagOverrides(cmd *cobra.Command) map[string]string { diff --git a/src/cli/flags/repo.go b/src/cli/flags/repo.go index ac84488b6..0344d2fbc 100644 --- a/src/cli/flags/repo.go +++ b/src/cli/flags/repo.go @@ -12,9 +12,8 @@ const ( AWSSessionTokenFN = "aws-session-token" // Corso Flags - PassphraseFN = "passphrase" - NewPassphraseFN = "new-passphrase" - SucceedIfExistsFN = "succeed-if-exists" + PassphraseFN = "passphrase" + NewPassphraseFN = "new-passphrase" ) var ( @@ -25,7 +24,6 @@ var ( AWSSessionTokenFV string PassphraseFV string NewPhasephraseFV string - SucceedIfExistsFV bool ) // AddMultipleBackupIDsFlag adds the --backups flag. diff --git a/src/cli/flags/s3.go b/src/cli/flags/s3.go index d0b97bbc8..8a807cc55 100644 --- a/src/cli/flags/s3.go +++ b/src/cli/flags/s3.go @@ -38,11 +38,6 @@ func AddS3BucketFlags(cmd *cobra.Command) { fs.StringVar(&EndpointFV, EndpointFN, "", "S3 service endpoint.") fs.BoolVar(&DoNotUseTLSFV, DoNotUseTLSFN, false, "Disable TLS (HTTPS)") fs.BoolVar(&DoNotVerifyTLSFV, DoNotVerifyTLSFN, false, "Disable TLS (HTTPS) certificate verification.") - - // In general, we don't want to expose this flag to users and have them mistake it - // for a broad-scale idempotency solution. We can un-hide it later the need arises. - fs.BoolVar(&SucceedIfExistsFV, SucceedIfExistsFN, false, "Exit with success if the repo has already been initialized.") - cobra.CheckErr(fs.MarkHidden("succeed-if-exists")) } func S3FlagOverrides(cmd *cobra.Command) map[string]string { diff --git a/src/cli/repo/filesystem.go b/src/cli/repo/filesystem.go index 37824a25e..9a2ad1d30 100644 --- a/src/cli/repo/filesystem.go +++ b/src/cli/repo/filesystem.go @@ -2,7 +2,6 @@ package repo import ( "github.com/alcionai/clues" - "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/alcionai/corso/src/cli/flags" @@ -110,10 +109,6 @@ func initFilesystemCmd(cmd *cobra.Command, args []string) error { ric := repository.InitConfig{RetentionOpts: retentionOpts} if err = r.Initialize(ctx, ric); err != nil { - if flags.SucceedIfExistsFV && errors.Is(err, repository.ErrorRepoAlreadyExists) { - return nil - } - return Only(ctx, clues.Stack(ErrInitializingRepo, err)) } diff --git a/src/cli/repo/s3.go b/src/cli/repo/s3.go index 412de3ea6..4cfc00753 100644 --- a/src/cli/repo/s3.go +++ b/src/cli/repo/s3.go @@ -4,7 +4,6 @@ import ( "strings" "github.com/alcionai/clues" - "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/alcionai/corso/src/cli/flags" @@ -132,10 +131,6 @@ func initS3Cmd(cmd *cobra.Command, args []string) error { ric := repository.InitConfig{RetentionOpts: retentionOpts} if err = r.Initialize(ctx, ric); err != nil { - if flags.SucceedIfExistsFV && errors.Is(err, repository.ErrorRepoAlreadyExists) { - return nil - } - return Only(ctx, clues.Stack(ErrInitializingRepo, err)) } diff --git a/src/cli/utils/flags_test.go b/src/cli/utils/flags_test.go index 1cf1c826d..436de81a4 100644 --- a/src/cli/utils/flags_test.go +++ b/src/cli/utils/flags_test.go @@ -103,7 +103,6 @@ func (suite *FlagUnitSuite) TestAddS3BucketFlags() { assert.Equal(t, "prefix1", flags.PrefixFV, flags.PrefixFN) assert.True(t, flags.DoNotUseTLSFV, flags.DoNotUseTLSFN) assert.True(t, flags.DoNotVerifyTLSFV, flags.DoNotVerifyTLSFN) - assert.True(t, flags.SucceedIfExistsFV, flags.SucceedIfExistsFN) }, } @@ -116,7 +115,6 @@ func (suite *FlagUnitSuite) TestAddS3BucketFlags() { "--" + flags.PrefixFN, "prefix1", "--" + flags.DoNotUseTLSFN, "--" + flags.DoNotVerifyTLSFN, - "--" + flags.SucceedIfExistsFN, }) err := cmd.Execute() @@ -130,7 +128,6 @@ func (suite *FlagUnitSuite) TestFilesystemFlags() { Use: "test", Run: func(cmd *cobra.Command, args []string) { assert.Equal(t, "/tmp/test", flags.FilesystemPathFV, flags.FilesystemPathFN) - assert.True(t, flags.SucceedIfExistsFV, flags.SucceedIfExistsFN) assert.Equal(t, "tenantID", flags.AzureClientTenantFV, flags.AzureClientTenantFN) assert.Equal(t, "clientID", flags.AzureClientIDFV, flags.AzureClientIDFN) assert.Equal(t, "secret", flags.AzureClientSecretFV, flags.AzureClientSecretFN) @@ -143,7 +140,6 @@ func (suite *FlagUnitSuite) TestFilesystemFlags() { cmd.SetArgs([]string{ "test", "--" + flags.FilesystemPathFN, "/tmp/test", - "--" + flags.SucceedIfExistsFN, "--" + flags.AzureClientIDFN, "clientID", "--" + flags.AzureClientTenantFN, "tenantID", "--" + flags.AzureClientSecretFN, "secret", diff --git a/src/internal/kopia/conn.go b/src/internal/kopia/conn.go index d79d37f41..436c865e9 100644 --- a/src/internal/kopia/conn.go +++ b/src/internal/kopia/conn.go @@ -149,12 +149,16 @@ func (w *conn) Initialize( RetentionPeriod: blobCfg.RetentionPeriod, } + var initErr error + if err = repo.Initialize(ctx, bst, &kopiaOpts, cfg.CorsoPassphrase); err != nil { - if errors.Is(err, repo.ErrAlreadyInitialized) { - return clues.StackWC(ctx, ErrorRepoAlreadyExists, err) + if !errors.Is(err, repo.ErrAlreadyInitialized) { + return clues.WrapWC(ctx, err, "initializing repo") } - return clues.WrapWC(ctx, err, "initializing repo") + logger.Ctx(ctx).Info("repo already exists, verifying repo config") + + initErr = clues.StackWC(ctx, ErrorRepoAlreadyExists, err) } err = w.commonConnect( @@ -166,7 +170,10 @@ func (w *conn) Initialize( cfg.CorsoPassphrase, defaultCompressor) if err != nil { - return err + // If the repo already exists then give some indication to that to help the + // user debug. For example, they could have called init again on a repo that + // already exists but accidentally used a different passphrase. + return clues.Stack(err, initErr) } if err := w.setDefaultConfigValues(ctx); err != nil { diff --git a/src/internal/kopia/conn_test.go b/src/internal/kopia/conn_test.go index 4513160a0..93c98479d 100644 --- a/src/internal/kopia/conn_test.go +++ b/src/internal/kopia/conn_test.go @@ -3,6 +3,7 @@ package kopia import ( "context" "math" + "strings" "testing" "time" @@ -15,6 +16,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "golang.org/x/exp/maps" "github.com/alcionai/corso/src/internal/common/ptr" strTD "github.com/alcionai/corso/src/internal/common/str/testdata" @@ -94,7 +96,7 @@ func TestWrapperIntegrationSuite(t *testing.T) { }) } -func (suite *WrapperIntegrationSuite) TestRepoExistsError() { +func (suite *WrapperIntegrationSuite) TestInitialize_SamePassphrase() { t := suite.T() repoNameHash := strTD.NewHashForRepoConfigName() @@ -110,6 +112,46 @@ func (suite *WrapperIntegrationSuite) TestRepoExistsError() { err = k.Close(ctx) require.NoError(t, err, clues.ToCore(err)) + err = k.Initialize(ctx, repository.Options{}, repository.Retention{}, repoNameHash) + assert.NoError(t, err, clues.ToCore(err)) +} + +func (suite *WrapperIntegrationSuite) TestInitialize_IncorrectPassphrase() { + t := suite.T() + repoNameHash := strTD.NewHashForRepoConfigName() + + ctx, flush := tester.NewContext(t) + defer flush() + + st1 := storeTD.NewFilesystemStorage(t) + k := NewConn(st1) + + err := k.Initialize(ctx, repository.Options{}, repository.Retention{}, repoNameHash) + require.NoError(t, err, clues.ToCore(err)) + + err = k.Close(ctx) + require.NoError(t, err, clues.ToCore(err)) + + // Hacky way to edit the existing passphrase for the repo so we can check that + // we get a sensible error back. + st2 := st1 + st2.Config = maps.Clone(st1.Config) + + var found bool + + for k, v := range st2.Config { + if strings.Contains(strings.ToLower(k), "passphrase") { + st2.Config[k] = v + "1" + found = true + + break + } + } + + require.True(t, found, "unable to update passphrase for test") + + k = NewConn(st2) + err = k.Initialize(ctx, repository.Options{}, repository.Retention{}, repoNameHash) assert.Error(t, err, clues.ToCore(err)) assert.ErrorIs(t, err, ErrorRepoAlreadyExists)