Update repo init for partial init case (#5060)

Unclear if this is precisely the path we want to take to handle possible
partial repo initialization so feel free to leave suggestions if a
different behavior would be better

Update the repo init logic to continue if the repo is marked as already existing. If it already exists then the process will try to update persistent config info in the repo. If the repo already exists but some connection credential is incorrect then stack errors such that both the reason for the connect failure and the fact that the repo already exists are both visible.

Manually tested attempting to initialize a repo that was already
initialized but using a different passphrase and got the output:
```text
Error: initializing repository: a repository was already initialized with that configuration: connecting to kopia repo: unable to create format manager: invalid repository password: repo already exists: repository already initialized
exit status 1
```

Attempting to initialize a repo again using the same passphrase resulted
in a success message with no errors reported

Unfortunately I can't really add unit tests to ensure the config ends up
the way we expect because I have no way to trigger an error partway through
kopia's init logic (or even just after it) without some non-trivial code
refactoring

---

#### Does this PR need a docs update or release note?

- [x] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [ ] ⛔ No

#### Type of change

- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Test Plan

- [x] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E

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+).

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 {

src/cli/flags/repo.go

@@ -14,7 +14,6 @@ const (
 	// Corso Flags
 	PassphraseFN    = "passphrase"
 	NewPassphraseFN = "new-passphrase"
-	SucceedIfExistsFN = "succeed-if-exists"
 )
 
 var (
@@ -25,7 +24,6 @@ var (
 	AWSSessionTokenFV    string
 	PassphraseFV         string
 	NewPhasephraseFV     string
-	SucceedIfExistsFV    bool
 )
 
 // AddMultipleBackupIDsFlag adds the --backups flag.

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 {

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))
 	}
 

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))
 	}
 

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",

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 {

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)