From 5258ef0f3603f149027fa37b0251cd9f45c831c7 Mon Sep 17 00:00:00 2001 From: Keepers Date: Thu, 28 Sep 2023 18:45:16 -0600 Subject: [PATCH] assert correct error on s3 conn bad configs e2e (#4387) #### Does this PR need a docs update or release note? - [x] :no_entry: No #### Type of change - [x] :bug: Bugfix - [x] :robot: Supportability/Tests #### Test Plan - [x] :green_heart: E2E --- src/cli/backup/helpers_test.go | 4 +- src/cli/config/account.go | 2 +- src/cli/config/config.go | 3 +- src/cli/config/config_test.go | 17 ++-- src/cli/repo/filesystem.go | 28 ++++--- src/cli/repo/filesystem_e2e_test.go | 6 +- src/cli/repo/repo.go | 5 ++ src/cli/repo/s3.go | 12 +-- src/cli/repo/s3_e2e_test.go | 117 ++++++++++++++------------- src/cli/restore/exchange_e2e_test.go | 4 +- src/cmd/s3checker/s3checker.go | 4 +- src/internal/kopia/conn.go | 2 +- src/internal/kopia/filesystem.go | 3 +- src/internal/kopia/s3.go | 4 +- src/pkg/repository/repository.go | 5 +- src/pkg/storage/common_test.go | 15 ++-- src/pkg/storage/filesystem.go | 6 +- src/pkg/storage/s3.go | 44 +++++----- src/pkg/storage/s3_test.go | 30 +++---- src/pkg/storage/storage.go | 9 ++- src/pkg/storage/storage_test.go | 12 +-- 21 files changed, 175 insertions(+), 157 deletions(-) diff --git a/src/cli/backup/helpers_test.go b/src/cli/backup/helpers_test.go index e7a59f361..8589d70d0 100644 --- a/src/cli/backup/helpers_test.go +++ b/src/cli/backup/helpers_test.go @@ -140,11 +140,9 @@ func prepM365Test( recorder = strings.Builder{} ) - sc, err := st.StorageConfig() + cfg, err := st.ToS3Config() require.NoError(t, err, clues.ToCore(err)) - cfg := sc.(*storage.S3Config) - force := map[string]string{ tconfig.TestCfgAccountProvider: account.ProviderM365.String(), tconfig.TestCfgStorageProvider: storage.ProviderS3.String(), diff --git a/src/cli/config/account.go b/src/cli/config/account.go index 8d87880d9..22a481b57 100644 --- a/src/cli/config/account.go +++ b/src/cli/config/account.go @@ -54,7 +54,7 @@ func configureAccount( if matchFromConfig { providerType := vpr.GetString(account.AccountProviderTypeKey) if providerType != account.ProviderM365.String() { - return acct, clues.New("unsupported account provider: " + providerType) + return acct, clues.New("unsupported account provider: [" + providerType + "]") } if err := mustMatchConfig(vpr, m365Overrides(overrides)); err != nil { diff --git a/src/cli/config/config.go b/src/cli/config/config.go index df8342ed1..6eab83fea 100644 --- a/src/cli/config/config.go +++ b/src/cli/config/config.go @@ -279,8 +279,7 @@ func getStorageAndAccountWithViper( // possibly read the prior config from a .corso file if readFromFile { - err = vpr.ReadInConfig() - if err != nil { + if err := vpr.ReadInConfig(); err != nil { if _, ok := err.(viper.ConfigFileNotFoundError); !ok { return config, clues.Wrap(err, "reading corso config file: "+vpr.ConfigFileUsed()) } diff --git a/src/cli/config/config_test.go b/src/cli/config/config_test.go index bccc79601..5d9fc42ce 100644 --- a/src/cli/config/config_test.go +++ b/src/cli/config/config_test.go @@ -356,10 +356,9 @@ func (suite *ConfigSuite) TestReadFromFlags() { m365Config, _ := repoDetails.Account.M365Config() - sc, err := repoDetails.Storage.StorageConfig() + s3Cfg, err := repoDetails.Storage.ToS3Config() require.NoError(t, err, "reading s3 config from storage", clues.ToCore(err)) - s3Cfg := sc.(*storage.S3Config) commonConfig, _ := repoDetails.Storage.CommonConfig() pass := commonConfig.Corso.CorsoPassphrase @@ -425,17 +424,21 @@ func (suite *ConfigIntegrationSuite) TestGetStorageAndAccount() { err = writeRepoConfigWithViper(vpr, s3Cfg, m365, repository.Options{}, "repoid") require.NoError(t, err, "writing repo config", clues.ToCore(err)) + require.Equal( + t, + account.ProviderM365.String(), + vpr.GetString(account.AccountProviderTypeKey), + "viper should have m365 as the account provider") + err = vpr.ReadInConfig() require.NoError(t, err, "reading repo config", clues.ToCore(err)) cfg, err := getStorageAndAccountWithViper(vpr, storage.ProviderS3, true, true, nil) require.NoError(t, err, "getting storage and account from config", clues.ToCore(err)) - sc, err := cfg.Storage.StorageConfig() + readS3Cfg, err := cfg.Storage.ToS3Config() require.NoError(t, err, "reading s3 config from storage", clues.ToCore(err)) - readS3Cfg := sc.(*storage.S3Config) - assert.Equal(t, readS3Cfg.Bucket, s3Cfg.Bucket) assert.Equal(t, readS3Cfg.Endpoint, s3Cfg.Endpoint) assert.Equal(t, readS3Cfg.Prefix, s3Cfg.Prefix) @@ -482,11 +485,9 @@ func (suite *ConfigIntegrationSuite) TestGetStorageAndAccount_noFileOnlyOverride cfg, err := getStorageAndAccountWithViper(vpr, storage.ProviderS3, false, true, overrides) require.NoError(t, err, "getting storage and account from config", clues.ToCore(err)) - sc, err := cfg.Storage.StorageConfig() + readS3Cfg, err := cfg.Storage.ToS3Config() require.NoError(t, err, "reading s3 config from storage", clues.ToCore(err)) - readS3Cfg := sc.(*storage.S3Config) - assert.Equal(t, readS3Cfg.Bucket, bkt) assert.Equal(t, cfg.RepoID, "") assert.Equal(t, readS3Cfg.Endpoint, end) diff --git a/src/cli/repo/filesystem.go b/src/cli/repo/filesystem.go index ef03d3657..40e8b05a5 100644 --- a/src/cli/repo/filesystem.go +++ b/src/cli/repo/filesystem.go @@ -96,13 +96,11 @@ func initFilesystemCmd(cmd *cobra.Command, args []string) error { cfg.Account.ID(), opt) - sc, err := cfg.Storage.StorageConfig() + storageCfg, err := cfg.Storage.ToFilesystemConfig() if err != nil { return Only(ctx, clues.Wrap(err, "Retrieving filesystem configuration")) } - storageCfg := sc.(*storage.FilesystemConfig) - m365, err := cfg.Account.M365Config() if err != nil { return Only(ctx, clues.Wrap(err, "Failed to parse m365 account config")) @@ -123,14 +121,20 @@ func initFilesystemCmd(cmd *cobra.Command, args []string) error { return nil } - return Only(ctx, clues.Wrap(err, "Failed to initialize a new filesystem repository")) + return Only(ctx, clues.Stack(ErrInitializingRepo, err)) } defer utils.CloseRepo(ctx, r) Infof(ctx, "Initialized a repository at path %s", storageCfg.Path) - if err = config.WriteRepoConfig(ctx, sc, m365, opt.Repo, r.GetID()); err != nil { + err = config.WriteRepoConfig( + ctx, + storageCfg, + m365, + opt.Repo, + r.GetID()) + if err != nil { return Only(ctx, clues.Wrap(err, "Failed to write repository configuration")) } @@ -181,13 +185,11 @@ func connectFilesystemCmd(cmd *cobra.Command, args []string) error { repoID = events.RepoIDNotFound } - sc, err := cfg.Storage.StorageConfig() + storageCfg, err := cfg.Storage.ToFilesystemConfig() if err != nil { return Only(ctx, clues.Wrap(err, "Retrieving filesystem configuration")) } - storageCfg := sc.(*storage.FilesystemConfig) - m365, err := cfg.Account.M365Config() if err != nil { return Only(ctx, clues.Wrap(err, "Failed to parse m365 account config")) @@ -206,14 +208,20 @@ func connectFilesystemCmd(cmd *cobra.Command, args []string) error { } if err := r.Connect(ctx); err != nil { - return Only(ctx, clues.Wrap(err, "Failed to connect to the filesystem repository")) + return Only(ctx, clues.Stack(ErrConnectingRepo, err)) } defer utils.CloseRepo(ctx, r) Infof(ctx, "Connected to repository at path %s", storageCfg.Path) - if err = config.WriteRepoConfig(ctx, sc, m365, opts.Repo, r.GetID()); err != nil { + err = config.WriteRepoConfig( + ctx, + storageCfg, + m365, + opts.Repo, + r.GetID()) + if err != nil { return Only(ctx, clues.Wrap(err, "Failed to write repository configuration")) } diff --git a/src/cli/repo/filesystem_e2e_test.go b/src/cli/repo/filesystem_e2e_test.go index 514d0120b..d7a28047c 100644 --- a/src/cli/repo/filesystem_e2e_test.go +++ b/src/cli/repo/filesystem_e2e_test.go @@ -56,9 +56,8 @@ func (suite *FilesystemE2ESuite) TestInitFilesystemCmd() { st := storeTD.NewFilesystemStorage(t) - sc, err := st.StorageConfig() + cfg, err := st.ToFilesystemConfig() require.NoError(t, err, clues.ToCore(err)) - cfg := sc.(*storage.FilesystemConfig) force := map[string]string{ tconfig.TestCfgStorageProvider: storage.ProviderFilesystem.String(), @@ -113,9 +112,8 @@ func (suite *FilesystemE2ESuite) TestConnectFilesystemCmd() { defer flush() st := storeTD.NewFilesystemStorage(t) - sc, err := st.StorageConfig() + cfg, err := st.ToFilesystemConfig() require.NoError(t, err, clues.ToCore(err)) - cfg := sc.(*storage.FilesystemConfig) force := map[string]string{ tconfig.TestCfgAccountProvider: account.ProviderM365.String(), diff --git a/src/cli/repo/repo.go b/src/cli/repo/repo.go index f5430613b..04b3a8413 100644 --- a/src/cli/repo/repo.go +++ b/src/cli/repo/repo.go @@ -20,6 +20,11 @@ const ( maintenanceCommand = "maintenance" ) +var ( + ErrConnectingRepo = clues.New("connecting repository") + ErrInitializingRepo = clues.New("initializing repository") +) + var repoCommands = []func(cmd *cobra.Command) *cobra.Command{ addS3Commands, addFilesystemCommands, diff --git a/src/cli/repo/s3.go b/src/cli/repo/s3.go index a450def04..253be0dfe 100644 --- a/src/cli/repo/s3.go +++ b/src/cli/repo/s3.go @@ -111,13 +111,11 @@ func initS3Cmd(cmd *cobra.Command, args []string) error { cfg.Account.ID(), opt) - sc, err := cfg.Storage.StorageConfig() + s3Cfg, err := cfg.Storage.ToS3Config() if err != nil { return Only(ctx, clues.Wrap(err, "Retrieving s3 configuration")) } - s3Cfg := sc.(*storage.S3Config) - if strings.HasPrefix(s3Cfg.Endpoint, "http://") || strings.HasPrefix(s3Cfg.Endpoint, "https://") { invalidEndpointErr := "endpoint doesn't support specifying protocol. " + "pass --disable-tls flag to use http:// instead of default https://" @@ -145,7 +143,7 @@ func initS3Cmd(cmd *cobra.Command, args []string) error { return nil } - return Only(ctx, clues.Wrap(err, "Failed to initialize a new S3 repository")) + return Only(ctx, clues.Stack(ErrInitializingRepo, err)) } defer utils.CloseRepo(ctx, r) @@ -194,13 +192,11 @@ func connectS3Cmd(cmd *cobra.Command, args []string) error { repoID = events.RepoIDNotFound } - sc, err := cfg.Storage.StorageConfig() + s3Cfg, err := cfg.Storage.ToS3Config() if err != nil { return Only(ctx, clues.Wrap(err, "Retrieving s3 configuration")) } - s3Cfg := sc.(*storage.S3Config) - m365, err := cfg.Account.M365Config() if err != nil { return Only(ctx, clues.Wrap(err, "Failed to parse m365 account config")) @@ -226,7 +222,7 @@ func connectS3Cmd(cmd *cobra.Command, args []string) error { } if err := r.Connect(ctx); err != nil { - return Only(ctx, clues.Wrap(err, "Failed to connect to the S3 repository")) + return Only(ctx, clues.Stack(ErrConnectingRepo, err)) } defer utils.CloseRepo(ctx, r) diff --git a/src/cli/repo/s3_e2e_test.go b/src/cli/repo/s3_e2e_test.go index 0bca84ca6..a9f50e277 100644 --- a/src/cli/repo/s3_e2e_test.go +++ b/src/cli/repo/s3_e2e_test.go @@ -8,10 +8,12 @@ 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/cli" "github.com/alcionai/corso/src/cli/config" cliTD "github.com/alcionai/corso/src/cli/testdata" + "github.com/alcionai/corso/src/internal/common/str" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/internal/tester/tconfig" "github.com/alcionai/corso/src/pkg/account" @@ -64,9 +66,8 @@ func (suite *S3E2ESuite) TestInitS3Cmd() { st := storeTD.NewPrefixedS3Storage(t) - sc, err := st.StorageConfig() + cfg, err := st.ToS3Config() require.NoError(t, err, clues.ToCore(err)) - cfg := sc.(*storage.S3Config) vpr, configFP := tconfig.MakeTempTestConfigClone(t, nil) if !test.hasConfigFile { @@ -102,10 +103,9 @@ func (suite *S3E2ESuite) TestInitMultipleTimes() { defer flush() st := storeTD.NewPrefixedS3Storage(t) - sc, err := st.StorageConfig() - require.NoError(t, err, clues.ToCore(err)) - cfg := sc.(*storage.S3Config) + cfg, err := st.ToS3Config() + require.NoError(t, err, clues.ToCore(err)) vpr, configFP := tconfig.MakeTempTestConfigClone(t, nil) @@ -134,11 +134,9 @@ func (suite *S3E2ESuite) TestInitS3Cmd_missingBucket() { st := storeTD.NewPrefixedS3Storage(t) - sc, err := st.StorageConfig() + cfg, err := st.ToS3Config() require.NoError(t, err, clues.ToCore(err)) - cfg := sc.(*storage.S3Config) - force := map[string]string{ tconfig.TestCfgBucket: "", } @@ -189,9 +187,9 @@ func (suite *S3E2ESuite) TestConnectS3Cmd() { defer flush() st := storeTD.NewPrefixedS3Storage(t) - sc, err := st.StorageConfig() + + cfg, err := st.ToS3Config() require.NoError(t, err, clues.ToCore(err)) - cfg := sc.(*storage.S3Config) force := map[string]string{ tconfig.TestCfgAccountProvider: account.ProviderM365.String(), @@ -234,58 +232,63 @@ func (suite *S3E2ESuite) TestConnectS3Cmd() { } } -func (suite *S3E2ESuite) TestConnectS3Cmd_BadBucket() { - t := suite.T() - ctx, flush := tester.NewContext(t) +func (suite *S3E2ESuite) TestConnectS3Cmd_badInputs() { + table := []struct { + name string + bucket string + prefix string + expectErr func(t *testing.T, err error) + }{ + { + name: "bucket", + bucket: "wrong", + expectErr: func(t *testing.T, err error) { + assert.ErrorIs(t, err, storage.ErrVerifyingConfigStorage, clues.ToCore(err)) + }, + }, + { + name: "prefix", + prefix: "wrong", + expectErr: func(t *testing.T, err error) { + assert.ErrorIs(t, err, storage.ErrVerifyingConfigStorage, clues.ToCore(err)) + }, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() - defer flush() + ctx, flush := tester.NewContext(t) + defer flush() - st := storeTD.NewPrefixedS3Storage(t) - sc, err := st.StorageConfig() - require.NoError(t, err, clues.ToCore(err)) + st := storeTD.NewPrefixedS3Storage(t) + cfg, err := st.ToS3Config() + require.NoError(t, err, clues.ToCore(err)) - cfg := sc.(*storage.S3Config) + bucket := str.First(test.bucket, cfg.Bucket) + prefix := str.First(test.prefix, cfg.Prefix) - vpr, configFP := tconfig.MakeTempTestConfigClone(t, nil) + over := map[string]string{} + acct := tconfig.NewM365Account(t) - ctx = config.SetViper(ctx, vpr) + maps.Copy(over, acct.Config) + over[account.AccountProviderTypeKey] = account.ProviderM365.String() + over[storage.StorageProviderTypeKey] = storage.ProviderS3.String() - cmd := cliTD.StubRootCmd( - "repo", "connect", "s3", - "--config-file", configFP, - "--bucket", "wrong", - "--prefix", cfg.Prefix) - cli.BuildCommandTree(cmd) + vpr, configFP := tconfig.MakeTempTestConfigClone(t, over) + ctx = config.SetViper(ctx, vpr) - // run the command - err = cmd.ExecuteContext(ctx) - require.Error(t, err, clues.ToCore(err)) -} - -func (suite *S3E2ESuite) TestConnectS3Cmd_BadPrefix() { - t := suite.T() - ctx, flush := tester.NewContext(t) - - defer flush() - - st := storeTD.NewPrefixedS3Storage(t) - sc, err := st.StorageConfig() - require.NoError(t, err, clues.ToCore(err)) - - cfg := sc.(*storage.S3Config) - - vpr, configFP := tconfig.MakeTempTestConfigClone(t, nil) - - ctx = config.SetViper(ctx, vpr) - - cmd := cliTD.StubRootCmd( - "repo", "connect", "s3", - "--config-file", configFP, - "--bucket", cfg.Bucket, - "--prefix", "wrong") - cli.BuildCommandTree(cmd) - - // run the command - err = cmd.ExecuteContext(ctx) - require.Error(t, err, clues.ToCore(err)) + cmd := cliTD.StubRootCmd( + "repo", "connect", "s3", + "--config-file", configFP, + "--bucket", bucket, + "--prefix", prefix) + cli.BuildCommandTree(cmd) + + // run the command + err = cmd.ExecuteContext(ctx) + require.Error(t, err, clues.ToCore(err)) + test.expectErr(t, err) + }) + } } diff --git a/src/cli/restore/exchange_e2e_test.go b/src/cli/restore/exchange_e2e_test.go index effbf14d8..36c6b8973 100644 --- a/src/cli/restore/exchange_e2e_test.go +++ b/src/cli/restore/exchange_e2e_test.go @@ -66,11 +66,9 @@ func (suite *RestoreExchangeE2ESuite) SetupSuite() { suite.acct = tconfig.NewM365Account(t) suite.st = storeTD.NewPrefixedS3Storage(t) - sc, err := suite.st.StorageConfig() + cfg, err := suite.st.ToS3Config() require.NoError(t, err, clues.ToCore(err)) - cfg := sc.(*storage.S3Config) - force := map[string]string{ tconfig.TestCfgAccountProvider: account.ProviderM365.String(), tconfig.TestCfgStorageProvider: storage.ProviderS3.String(), diff --git a/src/cmd/s3checker/s3checker.go b/src/cmd/s3checker/s3checker.go index 0c42b8aa5..7aa11e79a 100644 --- a/src/cmd/s3checker/s3checker.go +++ b/src/cmd/s3checker/s3checker.go @@ -197,13 +197,11 @@ func handleCheckerCommand(cmd *cobra.Command, args []string, f flags) error { return clues.Wrap(err, "getting storage config") } - sc, err := repoDetails.Storage.StorageConfig() + cfg, err := repoDetails.Storage.ToS3Config() if err != nil { return clues.Wrap(err, "getting S3 config") } - cfg := sc.(*storage.S3Config) - endpoint := defaultS3Endpoint if len(cfg.Endpoint) > 0 { endpoint = cfg.Endpoint diff --git a/src/internal/kopia/conn.go b/src/internal/kopia/conn.go index 001fd3f0f..ee8a9132e 100644 --- a/src/internal/kopia/conn.go +++ b/src/internal/kopia/conn.go @@ -205,7 +205,7 @@ func (w *conn) commonConnect( bst, password, kopiaOpts); err != nil { - return clues.Wrap(err, "connecting to repo").WithClues(ctx) + return clues.Wrap(err, "connecting to kopia repo").WithClues(ctx) } if err := w.open(ctx, cfgFile, password); err != nil { diff --git a/src/internal/kopia/filesystem.go b/src/internal/kopia/filesystem.go index 3081ac286..e67afa85e 100644 --- a/src/internal/kopia/filesystem.go +++ b/src/internal/kopia/filesystem.go @@ -16,12 +16,11 @@ func filesystemStorage( repoOpts repository.Options, s storage.Storage, ) (blob.Storage, error) { - cfg, err := s.StorageConfig() + fsCfg, err := s.ToFilesystemConfig() if err != nil { return nil, clues.Stack(err).WithClues(ctx) } - fsCfg := cfg.(*storage.FilesystemConfig) opts := filesystem.Options{ Path: fsCfg.Path, } diff --git a/src/internal/kopia/s3.go b/src/internal/kopia/s3.go index f4a379ada..b7dbbd5cf 100644 --- a/src/internal/kopia/s3.go +++ b/src/internal/kopia/s3.go @@ -20,13 +20,11 @@ func s3BlobStorage( repoOpts repository.Options, s storage.Storage, ) (blob.Storage, error) { - sc, err := s.StorageConfig() + cfg, err := s.ToS3Config() if err != nil { return nil, clues.Stack(err).WithClues(ctx) } - cfg := sc.(*storage.S3Config) - endpoint := defaultS3Endpoint if len(cfg.Endpoint) > 0 { endpoint = cfg.Endpoint diff --git a/src/pkg/repository/repository.go b/src/pkg/repository/repository.go index 2f836570c..277eb1bba 100644 --- a/src/pkg/repository/repository.go +++ b/src/pkg/repository/repository.go @@ -187,6 +187,8 @@ func (r *repository) Initialize( } }() + observe.Message(ctx, "Initializing repository") + kopiaRef := kopia.NewConn(r.Storage) if err := kopiaRef.Initialize(ctx, r.Opts.Repo, retentionOpts); err != nil { // replace common internal errors so that sdk users can check results with errors.Is() @@ -237,8 +239,7 @@ func (r *repository) Connect(ctx context.Context) (err error) { } }() - progressBar := observe.MessageWithCompletion(ctx, "Connecting to repository") - defer close(progressBar) + observe.Message(ctx, "Connecting to repository") kopiaRef := kopia.NewConn(r.Storage) if err := kopiaRef.Connect(ctx, r.Opts.Repo); err != nil { diff --git a/src/pkg/storage/common_test.go b/src/pkg/storage/common_test.go index 02668e611..e8b4a89ba 100644 --- a/src/pkg/storage/common_test.go +++ b/src/pkg/storage/common_test.go @@ -7,16 +7,17 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" + "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/credentials" "github.com/alcionai/corso/src/pkg/storage" ) -type CommonCfgSuite struct { - suite.Suite +type CommonCfgUnitSuite struct { + tester.Suite } -func TestCommonCfgSuite(t *testing.T) { - suite.Run(t, new(CommonCfgSuite)) +func TestCommonCfgUnitSuite(t *testing.T) { + suite.Run(t, &CommonCfgUnitSuite{Suite: tester.NewUnitSuite(t)}) } var goodCommonConfig = storage.CommonConfig{ @@ -25,7 +26,7 @@ var goodCommonConfig = storage.CommonConfig{ }, } -func (suite *CommonCfgSuite) TestCommonConfig_Config() { +func (suite *CommonCfgUnitSuite) TestCommonConfig_Config() { cfg := goodCommonConfig c, err := cfg.StringConfig() assert.NoError(suite.T(), err, clues.ToCore(err)) @@ -43,7 +44,7 @@ func (suite *CommonCfgSuite) TestCommonConfig_Config() { } } -func (suite *CommonCfgSuite) TestStorage_CommonConfig() { +func (suite *CommonCfgUnitSuite) TestStorage_CommonConfig() { t := suite.T() in := goodCommonConfig @@ -55,7 +56,7 @@ func (suite *CommonCfgSuite) TestStorage_CommonConfig() { assert.Equal(t, in.CorsoPassphrase, out.CorsoPassphrase) } -func (suite *CommonCfgSuite) TestStorage_CommonConfig_InvalidCases() { +func (suite *CommonCfgUnitSuite) TestStorage_CommonConfig_InvalidCases() { // missing required properties table := []struct { name string diff --git a/src/pkg/storage/filesystem.go b/src/pkg/storage/filesystem.go index ca4cfe098..08dacc62c 100644 --- a/src/pkg/storage/filesystem.go +++ b/src/pkg/storage/filesystem.go @@ -20,6 +20,10 @@ type FilesystemConfig struct { Path string } +func (s Storage) ToFilesystemConfig() (*FilesystemConfig, error) { + return buildFilesystemConfigFromMap(s.Config) +} + func buildFilesystemConfigFromMap(config map[string]string) (*FilesystemConfig, error) { c := &FilesystemConfig{} @@ -69,7 +73,7 @@ func (c *FilesystemConfig) ApplyConfigOverrides( if matchFromConfig { providerType := cast.ToString(g.Get(StorageProviderTypeKey)) if providerType != ProviderFilesystem.String() { - return clues.New("unsupported storage provider in config file: " + providerType) + return clues.New("unsupported storage provider in config file: [" + providerType + "]") } // This is matching override values from config file. diff --git a/src/pkg/storage/s3.go b/src/pkg/storage/s3.go index c689e77cd..7f2e8688f 100644 --- a/src/pkg/storage/s3.go +++ b/src/pkg/storage/s3.go @@ -62,6 +62,28 @@ var s3constToTomlKeyMap = map[string]string{ StorageProviderTypeKey: StorageProviderTypeKey, } +func (s Storage) ToS3Config() (*S3Config, error) { + return buildS3ConfigFromMap(s.Config) +} + +func buildS3ConfigFromMap(config map[string]string) (*S3Config, error) { + c := &S3Config{} + + if len(config) > 0 { + c.AccessKey = orEmptyString(config[keyS3AccessKey]) + c.SecretKey = orEmptyString(config[keyS3SecretKey]) + c.SessionToken = orEmptyString(config[keyS3SessionToken]) + + c.Bucket = orEmptyString(config[keyS3Bucket]) + c.Endpoint = orEmptyString(config[keyS3Endpoint]) + c.Prefix = orEmptyString(config[keyS3Prefix]) + c.DoNotUseTLS = str.ParseBool(config[keyS3DoNotUseTLS]) + c.DoNotVerifyTLS = str.ParseBool(config[keyS3DoNotVerifyTLS]) + } + + return c, c.validate() +} + func (c *S3Config) normalize() S3Config { return S3Config{ Bucket: common.NormalizeBucket(c.Bucket), @@ -91,24 +113,6 @@ func (c *S3Config) StringConfig() (map[string]string, error) { return cfg, cn.validate() } -func buildS3ConfigFromMap(config map[string]string) (*S3Config, error) { - c := &S3Config{} - - if len(config) > 0 { - c.AccessKey = orEmptyString(config[keyS3AccessKey]) - c.SecretKey = orEmptyString(config[keyS3SecretKey]) - c.SessionToken = orEmptyString(config[keyS3SessionToken]) - - c.Bucket = orEmptyString(config[keyS3Bucket]) - c.Endpoint = orEmptyString(config[keyS3Endpoint]) - c.Prefix = orEmptyString(config[keyS3Prefix]) - c.DoNotUseTLS = str.ParseBool(config[keyS3DoNotUseTLS]) - c.DoNotVerifyTLS = str.ParseBool(config[keyS3DoNotVerifyTLS]) - } - - return c, c.validate() -} - func (c S3Config) validate() error { check := map[string]string{ Bucket: c.Bucket, @@ -169,11 +173,11 @@ func (c *S3Config) ApplyConfigOverrides( if matchFromConfig { providerType := cast.ToString(kvg.Get(StorageProviderTypeKey)) if providerType != ProviderS3.String() { - return clues.New("unsupported storage provider: " + providerType) + return clues.New("unsupported storage provider: [" + providerType + "]") } if err := mustMatchConfig(kvg, s3constToTomlKeyMap, s3Overrides(overrides)); err != nil { - return clues.Wrap(err, "verifying s3 configs in corso config file") + return clues.Stack(err) } } } diff --git a/src/pkg/storage/s3_test.go b/src/pkg/storage/s3_test.go index 2a4b239f9..1e3a2e0ba 100644 --- a/src/pkg/storage/s3_test.go +++ b/src/pkg/storage/s3_test.go @@ -8,15 +8,16 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/credentials" ) -type S3CfgSuite struct { - suite.Suite +type S3CfgUnitSuite struct { + tester.Suite } -func TestS3CfgSuite(t *testing.T) { - suite.Run(t, new(S3CfgSuite)) +func TestS3CfgUnitSuite(t *testing.T) { + suite.Run(t, &S3CfgUnitSuite{Suite: tester.NewUnitSuite(t)}) } var ( @@ -41,7 +42,7 @@ var ( } ) -func (suite *S3CfgSuite) TestS3Config_Config() { +func (suite *S3CfgUnitSuite) TestS3Config_Config() { s3 := goodS3Config c, err := s3.StringConfig() @@ -60,16 +61,16 @@ func (suite *S3CfgSuite) TestS3Config_Config() { } } -func (suite *S3CfgSuite) TestStorage_S3Config() { +func (suite *S3CfgUnitSuite) TestStorage_S3Config() { t := suite.T() - in := goodS3Config + s, err := NewStorage(ProviderS3, &in) assert.NoError(t, err, clues.ToCore(err)) - sc, err := s.StorageConfig() + + out, err := s.ToS3Config() assert.NoError(t, err, clues.ToCore(err)) - out := sc.(*S3Config) assert.Equal(t, in.Bucket, out.Bucket) assert.Equal(t, in.Endpoint, out.Endpoint) assert.Equal(t, in.Prefix, out.Prefix) @@ -84,7 +85,7 @@ func makeTestS3Cfg(bkt, end, pre, access, secret, session string) S3Config { } } -func (suite *S3CfgSuite) TestStorage_S3Config_invalidCases() { +func (suite *S3CfgUnitSuite) TestStorage_S3Config_invalidCases() { // missing required properties table := []struct { name string @@ -118,13 +119,14 @@ func (suite *S3CfgSuite) TestStorage_S3Config_invalidCases() { st, err := NewStorage(ProviderUnknown, &goodS3Config) assert.NoError(t, err, clues.ToCore(err)) test.amend(st) - _, err = st.StorageConfig() - assert.Error(t, err) + + _, err = st.ToS3Config() + assert.Error(t, err, clues.ToCore(err)) }) } } -func (suite *S3CfgSuite) TestStorage_S3Config_StringConfig() { +func (suite *S3CfgUnitSuite) TestStorage_S3Config_StringConfig() { table := []struct { name string input S3Config @@ -178,7 +180,7 @@ func (suite *S3CfgSuite) TestStorage_S3Config_StringConfig() { } } -func (suite *S3CfgSuite) TestStorage_S3Config_Normalize() { +func (suite *S3CfgUnitSuite) TestStorage_S3Config_Normalize() { const ( prefixedBkt = "s3://bkt" normalBkt = "bkt" diff --git a/src/pkg/storage/storage.go b/src/pkg/storage/storage.go index 11b8863a1..c695ea992 100644 --- a/src/pkg/storage/storage.go +++ b/src/pkg/storage/storage.go @@ -9,6 +9,8 @@ import ( "github.com/alcionai/corso/src/internal/common" ) +var ErrVerifyingConfigStorage = clues.New("verifying configs in corso config file") + type ProviderType int //go:generate stringer -type=ProviderType -linecomment @@ -102,7 +104,7 @@ func (s Storage) StorageConfig() (Configurer, error) { return buildFilesystemConfigFromMap(s.Config) } - return nil, clues.New("unsupported storage provider: " + s.Provider.String()) + return nil, clues.New("unsupported storage provider: [" + s.Provider.String() + "]") } func NewStorageConfig(provider ProviderType) (Configurer, error) { @@ -113,7 +115,7 @@ func NewStorageConfig(provider ProviderType) (Configurer, error) { return &FilesystemConfig{}, nil } - return nil, clues.New("unsupported storage provider: " + provider.String()) + return nil, clues.New("unsupported storage provider: [" + provider.String() + "]") } type Getter interface { @@ -167,7 +169,8 @@ func mustMatchConfig( vv := cast.ToString(g.Get(tomlK)) if v != vv { - return clues.New("value of " + k + " (" + v + ") does not match corso configuration value (" + vv + ")") + err := clues.New("value of " + k + " (" + v + ") does not match corso configuration value (" + vv + ")") + return clues.Stack(ErrVerifyingConfigStorage, err) } } diff --git a/src/pkg/storage/storage_test.go b/src/pkg/storage/storage_test.go index 0d2cfbec6..095ea363c 100644 --- a/src/pkg/storage/storage_test.go +++ b/src/pkg/storage/storage_test.go @@ -6,6 +6,8 @@ import ( "github.com/alcionai/clues" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" + + "github.com/alcionai/corso/src/internal/tester" ) type testConfig struct { @@ -17,15 +19,15 @@ func (c testConfig) StringConfig() (map[string]string, error) { return map[string]string{"expect": c.expect}, c.err } -type StorageSuite struct { - suite.Suite +type StorageUnitSuite struct { + tester.Suite } -func TestStorageSuite(t *testing.T) { - suite.Run(t, new(StorageSuite)) +func TestStorageUnitSuite(t *testing.T) { + suite.Run(t, &StorageUnitSuite{Suite: tester.NewUnitSuite(t)}) } -func (suite *StorageSuite) TestNewStorage() { +func (suite *StorageUnitSuite) TestNewStorage() { table := []struct { name string p ProviderType