From 9705c9d2124e8fa124b7c1d9efa7a32fb65e979b Mon Sep 17 00:00:00 2001 From: Abhishek Pandey Date: Wed, 13 Sep 2023 17:49:50 +0530 Subject: [PATCH] More changes before piecemealing --- src/cli/backup/backup.go | 13 ++------ src/cli/backup/exchange.go | 7 ++--- src/cli/backup/groups.go | 7 ++--- src/cli/backup/helpers_test.go | 4 ++- src/cli/backup/onedrive.go | 7 ++--- src/cli/backup/sharepoint.go | 8 ++--- src/cli/cli.go | 3 +- src/cli/config/config.go | 10 +++--- src/cli/config/config_test.go | 35 ++++++++++++++++----- src/cli/config/storage.go | 8 ++--- src/cli/export/export.go | 4 +-- src/cli/repo/repo.go | 22 ++++++++++++- src/cli/repo/s3.go | 15 +++++---- src/cli/repo/s3_e2e_test.go | 22 +++++++++---- src/cli/restore/exchange_e2e_test.go | 4 ++- src/cli/restore/restore.go | 4 +-- src/cli/utils/utils.go | 6 ++-- src/cmd/longevity_test/longevity.go | 9 ++++-- src/cmd/s3checker/s3checker.go | 6 ++-- src/internal/kopia/s3.go | 4 ++- src/pkg/storage/s3.go | 47 ++++++++++++++-------------- src/pkg/storage/s3_test.go | 6 ++-- src/pkg/storage/storage.go | 42 +++++++++++-------------- 23 files changed, 164 insertions(+), 129 deletions(-) diff --git a/src/cli/backup/backup.go b/src/cli/backup/backup.go index d141d22ba..8a084652b 100644 --- a/src/cli/backup/backup.go +++ b/src/cli/backup/backup.go @@ -10,7 +10,6 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" - "github.com/alcionai/corso/src/cli/config" "github.com/alcionai/corso/src/cli/flags" . "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/cli/repo" @@ -291,16 +290,12 @@ func genericDeleteCommand( ctx := clues.Add(cmd.Context(), "delete_backup_id", bID) - // Let it return both provider and overrides for now? - // That way we can stop config pkg from being included everywhere. - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - - overrides, err := repo.GetStorageOverrides(ctx, cmd, provider) + storageProvider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cmd) if err != nil { return Only(ctx, err) } - r, _, _, _, err := utils.GetAccountAndConnect(ctx, pst, provider, overrides) + r, _, _, _, err := utils.GetAccountAndConnect(ctx, pst, storageProvider, overrides) if err != nil { return Only(ctx, err) } @@ -326,9 +321,7 @@ func genericListCommand( ) error { ctx := cmd.Context() - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - - overrides, err := repo.GetStorageOverrides(ctx, cmd, provider) + provider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cmd) if err != nil { return Only(ctx, err) } diff --git a/src/cli/backup/exchange.go b/src/cli/backup/exchange.go index 5c650a547..a9e61087d 100644 --- a/src/cli/backup/exchange.go +++ b/src/cli/backup/exchange.go @@ -8,7 +8,6 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" - "github.com/alcionai/corso/src/cli/config" "github.com/alcionai/corso/src/cli/flags" . "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/cli/repo" @@ -169,8 +168,7 @@ func createExchangeCmd(cmd *cobra.Command, args []string) error { return err } - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - overrides, err := repo.GetStorageOverrides(ctx, cmd, provider) + provider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cmd) if err != nil { return Only(ctx, err) } @@ -284,8 +282,7 @@ func detailsExchangeCmd(cmd *cobra.Command, args []string) error { ctx := cmd.Context() opts := utils.MakeExchangeOpts(cmd) - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - overrides, err := repo.GetStorageOverrides(ctx, cmd, provider) + provider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cmd) if err != nil { return Only(ctx, err) } diff --git a/src/cli/backup/groups.go b/src/cli/backup/groups.go index a93ac0c87..e088668a1 100644 --- a/src/cli/backup/groups.go +++ b/src/cli/backup/groups.go @@ -10,7 +10,6 @@ import ( "github.com/spf13/pflag" "golang.org/x/exp/slices" - "github.com/alcionai/corso/src/cli/config" "github.com/alcionai/corso/src/cli/flags" . "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/cli/repo" @@ -155,8 +154,7 @@ func createGroupsCmd(cmd *cobra.Command, args []string) error { return err } - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - overrides, err := repo.GetStorageOverrides(ctx, cmd, provider) + provider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cmd) if err != nil { return Only(ctx, err) } @@ -233,8 +231,7 @@ func detailsGroupsCmd(cmd *cobra.Command, args []string) error { ctx := cmd.Context() opts := utils.MakeGroupsOpts(cmd) - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - overrides, err := repo.GetStorageOverrides(ctx, cmd, provider) + provider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cmd) if err != nil { return Only(ctx, err) } diff --git a/src/cli/backup/helpers_test.go b/src/cli/backup/helpers_test.go index a54019c1c..7f430c4db 100644 --- a/src/cli/backup/helpers_test.go +++ b/src/cli/backup/helpers_test.go @@ -140,9 +140,11 @@ func prepM365Test( recorder = strings.Builder{} ) - cfg, err := st.S3Config() + c, err := st.StorageConfig() require.NoError(t, err, clues.ToCore(err)) + cfg := c.(storage.S3Config) + force := map[string]string{ tconfig.TestCfgAccountProvider: account.ProviderM365.String(), tconfig.TestCfgStorageProvider: storage.ProviderS3.String(), diff --git a/src/cli/backup/onedrive.go b/src/cli/backup/onedrive.go index cab9eb2e1..3e35529b6 100644 --- a/src/cli/backup/onedrive.go +++ b/src/cli/backup/onedrive.go @@ -8,7 +8,6 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" - "github.com/alcionai/corso/src/cli/config" "github.com/alcionai/corso/src/cli/flags" . "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/cli/repo" @@ -150,8 +149,7 @@ func createOneDriveCmd(cmd *cobra.Command, args []string) error { return err } - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - overrides, err := repo.GetStorageOverrides(ctx, cmd, provider) + provider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cmd) if err != nil { return Only(ctx, err) } @@ -242,8 +240,7 @@ func detailsOneDriveCmd(cmd *cobra.Command, args []string) error { ctx := cmd.Context() opts := utils.MakeOneDriveOpts(cmd) - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - overrides, err := repo.GetStorageOverrides(ctx, cmd, provider) + provider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cmd) if err != nil { return Only(ctx, err) } diff --git a/src/cli/backup/sharepoint.go b/src/cli/backup/sharepoint.go index fc0b1bc21..fbcde66c5 100644 --- a/src/cli/backup/sharepoint.go +++ b/src/cli/backup/sharepoint.go @@ -9,7 +9,6 @@ import ( "github.com/spf13/pflag" "golang.org/x/exp/slices" - "github.com/alcionai/corso/src/cli/config" "github.com/alcionai/corso/src/cli/flags" . "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/cli/repo" @@ -160,9 +159,7 @@ func createSharePointCmd(cmd *cobra.Command, args []string) error { return err } - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - - overrides, err := repo.GetStorageOverrides(ctx, cmd, provider) + provider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cmd) if err != nil { return Only(ctx, err) } @@ -327,8 +324,7 @@ func detailsSharePointCmd(cmd *cobra.Command, args []string) error { ctx := cmd.Context() opts := utils.MakeSharePointOpts(cmd) - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - overrides, err := repo.GetStorageOverrides(ctx, cmd, provider) + provider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cmd) if err != nil { return Only(ctx, err) } diff --git a/src/cli/cli.go b/src/cli/cli.go index 6a82ad75f..d54271a90 100644 --- a/src/cli/cli.go +++ b/src/cli/cli.go @@ -70,8 +70,7 @@ func preRun(cc *cobra.Command, args []string) error { } if !slices.Contains(avoidTheseDescription, cc.Short) { - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - overrides, err := repo.GetStorageOverrides(ctx, cc, provider) + provider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cc) if err != nil { return err } diff --git a/src/cli/config/config.go b/src/cli/config/config.go index d8e731353..3b22a99fa 100644 --- a/src/cli/config/config.go +++ b/src/cli/config/config.go @@ -255,7 +255,7 @@ func writeRepoConfigWithViper( // data sources (config file, env vars, flag overrides) and the config file. func GetConfigRepoDetails( ctx context.Context, - provider string, + provider storage.ProviderType, readFromFile bool, mustMatchFromConfig bool, overrides map[string]string, @@ -271,7 +271,7 @@ func GetConfigRepoDetails( // struct for testing. func getStorageAndAccountWithViper( vpr *viper.Viper, - provider string, + provider storage.ProviderType, readFromFile bool, mustMatchFromConfig bool, overrides map[string]string, @@ -376,13 +376,13 @@ func requireProps(props map[string]string) error { // Storage provider is not a flag. It can only be sourced from config file. // Only exceptions are the commands that create a new repo. // This is needed to figure out which storage overrides to use. -func GetStorageProviderFromConfigFile(ctx context.Context) (string, error) { +func GetStorageProviderFromConfigFile(ctx context.Context) (storage.ProviderType, error) { vpr := GetViper(ctx) provider := vpr.GetString(StorageProviderTypeKey) if provider != storage.ProviderS3.String() { - return storage.ProviderUnknown.String(), clues.New("unsupported storage provider: " + provider) + return storage.ProviderUnknown, clues.New("unsupported storage provider: " + provider) } - return provider, nil + return storage.StringToProviderType[provider], nil } diff --git a/src/cli/config/config_test.go b/src/cli/config/config_test.go index b697f0080..29bbd18da 100644 --- a/src/cli/config/config_test.go +++ b/src/cli/config/config_test.go @@ -107,14 +107,23 @@ func (suite *ConfigSuite) TestReadRepoConfigBasic() { err = vpr.ReadInConfig() require.NoError(t, err, "reading repo config", clues.ToCore(err)) - s3Cfg, err := s3ConfigsFromViper(vpr) + // Unset AWS env vars so that we can test reading creds from config file + os.Unsetenv(credentials.AWSAccessKeyID) + os.Unsetenv(credentials.AWSSecretAccessKey) + os.Unsetenv(credentials.AWSSessionToken) + + sc, err := storage.S3Config{}.FetchConfigFromStore(vpr, true, true, nil) + require.NoError(t, err, clues.ToCore(err)) + + s3Cfg := sc.(storage.S3Config) + require.NoError(t, err, clues.ToCore(err)) assert.Equal(t, b, s3Cfg.Bucket) assert.Equal(t, "test-prefix/", s3Cfg.Prefix) assert.Equal(t, disableTLS, strconv.FormatBool(s3Cfg.DoNotUseTLS)) assert.Equal(t, disableTLSVerification, strconv.FormatBool(s3Cfg.DoNotVerifyTLS)) - s3Cfg, err = s3CredsFromViper(vpr, s3Cfg) + // s3Cfg, err = s3CredsFromViper(vpr, s3Cfg) require.NoError(t, err, clues.ToCore(err)) assert.Equal(t, accKey, s3Cfg.AWS.AccessKey) assert.Equal(t, secret, s3Cfg.AWS.SecretKey) @@ -160,7 +169,11 @@ func (suite *ConfigSuite) TestWriteReadConfig() { err = vpr.ReadInConfig() require.NoError(t, err, "reading repo config", clues.ToCore(err)) - readS3Cfg, err := s3ConfigsFromViper(vpr) + sc, err := storage.S3Config{}.FetchConfigFromStore(vpr, true, true, nil) + require.NoError(t, err, clues.ToCore(err)) + + readS3Cfg := sc.(storage.S3Config) + require.NoError(t, err, clues.ToCore(err)) assert.Equal(t, readS3Cfg.Bucket, s3Cfg.Bucket) assert.Equal(t, readS3Cfg.DoNotUseTLS, s3Cfg.DoNotUseTLS) @@ -326,12 +339,14 @@ func (suite *ConfigSuite) TestReadFromFlags() { repoDetails, err := getStorageAndAccountWithViper( vpr, + storage.ProviderS3, true, false, overrides) m365Config, _ := repoDetails.Account.M365Config() - s3Cfg, _ := repoDetails.Storage.S3Config() + cfg, _ := repoDetails.Storage.StorageConfig() + s3Cfg, _ := cfg.(storage.S3Config) commonConfig, _ := repoDetails.Storage.CommonConfig() pass := commonConfig.Corso.CorsoPassphrase @@ -400,11 +415,13 @@ func (suite *ConfigIntegrationSuite) TestGetStorageAndAccount() { err = vpr.ReadInConfig() require.NoError(t, err, "reading repo config", clues.ToCore(err)) - cfg, err := getStorageAndAccountWithViper(vpr, true, true, nil) + cfg, err := getStorageAndAccountWithViper(vpr, storage.ProviderS3, true, true, nil) require.NoError(t, err, "getting storage and account from config", clues.ToCore(err)) - readS3Cfg, err := cfg.Storage.S3Config() + sc, err := cfg.Storage.StorageConfig() 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) @@ -448,11 +465,13 @@ func (suite *ConfigIntegrationSuite) TestGetStorageAndAccount_noFileOnlyOverride StorageProviderTypeKey: storage.ProviderS3.String(), } - cfg, err := getStorageAndAccountWithViper(vpr, false, true, overrides) + cfg, err := getStorageAndAccountWithViper(vpr, storage.ProviderS3, false, true, overrides) require.NoError(t, err, "getting storage and account from config", clues.ToCore(err)) - readS3Cfg, err := cfg.Storage.S3Config() + sc, err := cfg.Storage.StorageConfig() 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/config/storage.go b/src/cli/config/storage.go index bad20d514..50b8f5514 100644 --- a/src/cli/config/storage.go +++ b/src/cli/config/storage.go @@ -17,7 +17,7 @@ import ( // viper properties and manual overrides. func configureStorage( vpr *viper.Viper, - provider string, + provider storage.ProviderType, readConfigFromViper bool, matchFromConfig bool, overrides map[string]string, @@ -29,7 +29,8 @@ func configureStorage( storageCfg, _ := storage.NewStorageConfig(provider) - err = storageCfg.FetchConfigFromStore( + // Rename this. It's not just fetch config from store. + storageCfg, err = storageCfg.FetchConfigFromStore( vpr, readConfigFromViper, matchFromConfig, @@ -63,8 +64,7 @@ func configureStorage( } // build the storage - store, err = storage.NewStorage( - storage.StringToEnum(provider), storageCfg, cCfg) + store, err = storage.NewStorage(provider, storageCfg, cCfg) if err != nil { return store, clues.Wrap(err, "configuring repository storage") } diff --git a/src/cli/export/export.go b/src/cli/export/export.go index 571d4f938..714f8741f 100644 --- a/src/cli/export/export.go +++ b/src/cli/export/export.go @@ -8,7 +8,6 @@ import ( "github.com/alcionai/clues" "github.com/spf13/cobra" - "github.com/alcionai/corso/src/cli/config" . "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/cli/repo" "github.com/alcionai/corso/src/cli/utils" @@ -71,8 +70,7 @@ func runExport( sel selectors.Selector, backupID, serviceName string, ) error { - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - overrides, err := repo.GetStorageOverrides(ctx, cmd, provider) + provider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cmd) if err != nil { return Only(ctx, err) } diff --git a/src/cli/repo/repo.go b/src/cli/repo/repo.go index fd83f7f97..7ec4fb674 100644 --- a/src/cli/repo/repo.go +++ b/src/cli/repo/repo.go @@ -8,6 +8,7 @@ import ( "github.com/spf13/cobra" "golang.org/x/exp/maps" + "github.com/alcionai/corso/src/cli/config" "github.com/alcionai/corso/src/cli/flags" "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/cli/utils" @@ -125,7 +126,7 @@ func handleMaintenanceCmd(cmd *cobra.Command, args []string) error { // Change this to override too? r, _, err := utils.AccountConnectAndWriteRepoConfig( - ctx, path.UnknownService, storage.ProviderS3.String(), S3Overrides(cmd)) + ctx, path.UnknownService, storage.ProviderS3, S3Overrides(cmd)) if err != nil { return print.Only(ctx, err) } @@ -183,3 +184,22 @@ func GetStorageOverrides( return overrides, nil } + +func GetStorageProviderAndOverrides( + ctx context.Context, + cmd *cobra.Command, +) (storage.ProviderType, map[string]string, error) { + provider, err := config.GetStorageProviderFromConfigFile(ctx) + if err != nil { + return provider, nil, clues.Stack(err) + } + + overrides := map[string]string{} + + switch provider { + case storage.ProviderS3: + overrides = S3Overrides(cmd) + } + + return provider, overrides, nil +} diff --git a/src/cli/repo/s3.go b/src/cli/repo/s3.go index e2a03d158..34a99d75e 100644 --- a/src/cli/repo/s3.go +++ b/src/cli/repo/s3.go @@ -91,7 +91,7 @@ func initS3Cmd(cmd *cobra.Command, args []string) error { // s3 values from flags s3Override := S3Overrides(cmd) - cfg, err := config.GetConfigRepoDetails(ctx, storage.ProviderS3.String(), true, false, s3Override) + cfg, err := config.GetConfigRepoDetails(ctx, storage.ProviderS3, true, false, s3Override) if err != nil { return Only(ctx, err) } @@ -112,11 +112,10 @@ func initS3Cmd(cmd *cobra.Command, args []string) error { cfg.Account.ID(), opt) - //s3Cfg, err := cfg.Storage.S3Config() // why not let it return configurer? - storageCfg, err := cfg.Storage.GetStorageConfig() - // if err != nil { - // return Only(ctx, clues.Wrap(err, "Retrieving s3 configuration")) - // } + storageCfg, err := cfg.Storage.StorageConfig() + if err != nil { + return Only(ctx, clues.Wrap(err, "Retrieving s3 configuration")) + } // BUG: This should be moved to validate() // if strings.HasPrefix(s3Cfg.Endpoint, "http://") || strings.HasPrefix(s3Cfg.Endpoint, "https://") { @@ -180,7 +179,7 @@ func connectS3Cmd(cmd *cobra.Command, args []string) error { // s3 values from flags s3Override := S3Overrides(cmd) - cfg, err := config.GetConfigRepoDetails(ctx, storage.ProviderS3.String(), true, true, s3Override) + cfg, err := config.GetConfigRepoDetails(ctx, storage.ProviderS3, true, true, s3Override) if err != nil { return Only(ctx, err) } @@ -190,7 +189,7 @@ func connectS3Cmd(cmd *cobra.Command, args []string) error { repoID = events.RepoIDNotFound } - s3Cfg, err := cfg.Storage.GetStorageConfig() + s3Cfg, err := cfg.Storage.StorageConfig() if err != nil { return Only(ctx, clues.Wrap(err, "Retrieving s3 configuration")) } diff --git a/src/cli/repo/s3_e2e_test.go b/src/cli/repo/s3_e2e_test.go index 28dc7c3c7..963712555 100644 --- a/src/cli/repo/s3_e2e_test.go +++ b/src/cli/repo/s3_e2e_test.go @@ -63,9 +63,11 @@ func (suite *S3E2ESuite) TestInitS3Cmd() { defer flush() st := storeTD.NewPrefixedS3Storage(t) - cfg, err := st.S3Config() + sc, err := st.StorageConfig() require.NoError(t, err, clues.ToCore(err)) + cfg := sc.(storage.S3Config) + vpr, configFP := tconfig.MakeTempTestConfigClone(t, nil) if !test.hasConfigFile { // Ideally we could use `/dev/null`, but you need a @@ -100,9 +102,11 @@ func (suite *S3E2ESuite) TestInitMultipleTimes() { defer flush() st := storeTD.NewPrefixedS3Storage(t) - cfg, err := st.S3Config() + 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) @@ -129,9 +133,10 @@ func (suite *S3E2ESuite) TestInitS3Cmd_missingBucket() { defer flush() st := storeTD.NewPrefixedS3Storage(t) - cfg, err := st.S3Config() + sc, err := st.StorageConfig() require.NoError(t, err, clues.ToCore(err)) + cfg := sc.(storage.S3Config) force := map[string]string{ tconfig.TestCfgBucket: "", } @@ -182,9 +187,11 @@ func (suite *S3E2ESuite) TestConnectS3Cmd() { defer flush() st := storeTD.NewPrefixedS3Storage(t) - cfg, err := st.S3Config() + sc, err := st.StorageConfig() 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(), @@ -230,9 +237,10 @@ func (suite *S3E2ESuite) TestConnectS3Cmd_BadBucket() { defer flush() st := storeTD.NewPrefixedS3Storage(t) - cfg, err := st.S3Config() + 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) @@ -256,9 +264,11 @@ func (suite *S3E2ESuite) TestConnectS3Cmd_BadPrefix() { defer flush() st := storeTD.NewPrefixedS3Storage(t) - cfg, err := st.S3Config() + 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) diff --git a/src/cli/restore/exchange_e2e_test.go b/src/cli/restore/exchange_e2e_test.go index 4b8b79e75..601d7798a 100644 --- a/src/cli/restore/exchange_e2e_test.go +++ b/src/cli/restore/exchange_e2e_test.go @@ -66,9 +66,11 @@ func (suite *RestoreExchangeE2ESuite) SetupSuite() { suite.acct = tconfig.NewM365Account(t) suite.st = storeTD.NewPrefixedS3Storage(t) - cfg, err := suite.st.S3Config() + sc, err := suite.st.StorageConfig() 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/restore/restore.go b/src/cli/restore/restore.go index dc4e3d1b0..d8cd908d6 100644 --- a/src/cli/restore/restore.go +++ b/src/cli/restore/restore.go @@ -8,7 +8,6 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" - "github.com/alcionai/corso/src/cli/config" "github.com/alcionai/corso/src/cli/flags" . "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/cli/repo" @@ -104,8 +103,7 @@ func runRestore( sel selectors.Selector, backupID, serviceName string, ) error { - provider, _ := config.GetStorageProviderFromConfigFile(ctx) - overrides, err := repo.GetStorageOverrides(ctx, cmd, provider) + provider, overrides, err := repo.GetStorageProviderAndOverrides(ctx, cmd) if err != nil { return Only(ctx, err) } diff --git a/src/cli/utils/utils.go b/src/cli/utils/utils.go index 0ac27a261..91d52ff92 100644 --- a/src/cli/utils/utils.go +++ b/src/cli/utils/utils.go @@ -24,7 +24,7 @@ var ErrNotYetImplemented = clues.New("not yet implemented") func GetAccountAndConnect( ctx context.Context, pst path.ServiceType, - provider string, + provider storage.ProviderType, overrides map[string]string, ) (repository.Repository, *storage.Storage, *account.Account, *control.Options, error) { cfg, err := config.GetConfigRepoDetails(ctx, provider, true, true, overrides) @@ -56,7 +56,7 @@ func GetAccountAndConnect( func AccountConnectAndWriteRepoConfig( ctx context.Context, pst path.ServiceType, - provider string, + provider storage.ProviderType, overrides map[string]string, ) (repository.Repository, *account.Account, error) { r, stg, acc, opts, err := GetAccountAndConnect(ctx, pst, provider, overrides) @@ -65,7 +65,7 @@ func AccountConnectAndWriteRepoConfig( return nil, nil, err } - storageCfg, err := stg.GetStorageConfig() + storageCfg, err := stg.StorageConfig() if err != nil { logger.CtxErr(ctx, err).Info("getting storage configuration") diff --git a/src/cmd/longevity_test/longevity.go b/src/cmd/longevity_test/longevity.go index 6f36057cc..0819b8c86 100644 --- a/src/cmd/longevity_test/longevity.go +++ b/src/cmd/longevity_test/longevity.go @@ -16,6 +16,7 @@ import ( "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/repository" + "github.com/alcionai/corso/src/pkg/storage" "github.com/alcionai/corso/src/pkg/store" ) @@ -29,7 +30,11 @@ func deleteBackups( ) ([]string, error) { ctx = clues.Add(ctx, "cutoff_days", deletionDays) - provider, _ := config.GetStorageProviderFromConfigFile(ctx) + provider, err := config.GetStorageProviderFromConfigFile(ctx) + if err != nil { + return nil, err + } + r, _, _, _, err := utils.GetAccountAndConnect(ctx, service, provider, nil) if err != nil { return nil, clues.Wrap(err, "connecting to account").WithClues(ctx) @@ -88,7 +93,7 @@ func pitrListBackups( // TODO(ashmrtn): This may be moved into CLI layer at some point when we add // flags for opening a repo at a point in time. - cfg, err := config.GetConfigRepoDetails(ctx, true, true, nil) + cfg, err := config.GetConfigRepoDetails(ctx, storage.ProviderS3, true, true, nil) if err != nil { return clues.Wrap(err, "getting config info") } diff --git a/src/cmd/s3checker/s3checker.go b/src/cmd/s3checker/s3checker.go index 6413f7d83..9c6d2e573 100644 --- a/src/cmd/s3checker/s3checker.go +++ b/src/cmd/s3checker/s3checker.go @@ -187,16 +187,18 @@ func handleCheckerCommand(cmd *cobra.Command, args []string, f flags) error { storage.Prefix: f.bucketPrefix, } - repoDetails, err := config.GetConfigRepoDetails(ctx, false, false, overrides) + repoDetails, err := config.GetConfigRepoDetails(ctx, storage.ProviderS3, false, false, overrides) if err != nil { return clues.Wrap(err, "getting storage config") } - cfg, err := repoDetails.Storage.S3Config() + c, err := repoDetails.Storage.StorageConfig() if err != nil { return clues.Wrap(err, "getting S3 config") } + cfg := c.(storage.S3Config) + endpoint := defaultS3Endpoint if len(cfg.Endpoint) > 0 { endpoint = cfg.Endpoint diff --git a/src/internal/kopia/s3.go b/src/internal/kopia/s3.go index a4f7524e2..0fc0966e6 100644 --- a/src/internal/kopia/s3.go +++ b/src/internal/kopia/s3.go @@ -8,6 +8,7 @@ import ( "github.com/kopia/kopia/repo/blob/s3" "github.com/alcionai/corso/src/pkg/control/repository" + "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/storage" ) @@ -20,7 +21,7 @@ func s3BlobStorage( repoOpts repository.Options, s storage.Storage, ) (blob.Storage, error) { - cfg, err := s.GetStorageConfig() + cfg, err := s.StorageConfig() if err != nil { return nil, clues.Stack(err).WithClues(ctx) } @@ -33,6 +34,7 @@ func s3BlobStorage( endpoint = s3Cfg.Endpoint } + logger.Ctx(ctx).Infow("aws creds", "key", s3Cfg.AccessKey) opts := s3.Options{ BucketName: s3Cfg.Bucket, Endpoint: endpoint, diff --git a/src/pkg/storage/s3.go b/src/pkg/storage/s3.go index 33aaf4592..aba918c88 100644 --- a/src/pkg/storage/s3.go +++ b/src/pkg/storage/s3.go @@ -68,44 +68,45 @@ func (c S3Config) Normalize() S3Config { } // No need to return error here. Viper returns empty values. -func s3ConfigsFromStore(kvs KVStorer) S3Config { +func s3ConfigsFromStore(kvg KVStoreGetter) S3Config { var s3Config S3Config - s3Config.Bucket = cast.ToString(kvs.Get(BucketNameKey)) - s3Config.Endpoint = cast.ToString(kvs.Get(EndpointKey)) - s3Config.Prefix = cast.ToString(kvs.Get(PrefixKey)) - s3Config.DoNotUseTLS = cast.ToBool(kvs.Get(DisableTLSKey)) - s3Config.DoNotVerifyTLS = cast.ToBool(kvs.Get(DisableTLSVerificationKey)) + s3Config.Bucket = cast.ToString(kvg.Get(BucketNameKey)) + s3Config.Endpoint = cast.ToString(kvg.Get(EndpointKey)) + s3Config.Prefix = cast.ToString(kvg.Get(PrefixKey)) + s3Config.DoNotUseTLS = cast.ToBool(kvg.Get(DisableTLSKey)) + s3Config.DoNotVerifyTLS = cast.ToBool(kvg.Get(DisableTLSVerificationKey)) return s3Config } func s3CredsFromStore( - kvs KVStorer, + kvg KVStoreGetter, s3Config S3Config, ) S3Config { - s3Config.AccessKey = cast.ToString(kvs.Get(AccessKey)) - s3Config.SecretKey = cast.ToString(kvs.Get(SecretAccessKey)) - s3Config.SessionToken = cast.ToString(kvs.Get(SessionToken)) + s3Config.AccessKey = cast.ToString(kvg.Get(AccessKey)) + s3Config.SecretKey = cast.ToString(kvg.Get(SecretAccessKey)) + s3Config.SessionToken = cast.ToString(kvg.Get(SessionToken)) return s3Config } -var _ StorageConfigurer = S3Config{} +var _ Configurer = S3Config{} func (c S3Config) FetchConfigFromStore( - kvs KVStorer, + kvg KVStoreGetter, readConfigFromStore bool, matchFromConfig bool, overrides map[string]string, -) error { +) (Configurer, error) { var ( s3Cfg S3Config err error ) if readConfigFromStore { - s3Cfg = s3ConfigsFromStore(kvs) + s3Cfg = s3ConfigsFromStore(kvg) + if b, ok := overrides[Bucket]; ok { overrides[Bucket] = common.NormalizeBucket(b) } @@ -115,19 +116,19 @@ func (c S3Config) FetchConfigFromStore( } if matchFromConfig { - providerType := cast.ToString(kvs.Get(StorageProviderTypeKey)) + providerType := cast.ToString(kvg.Get(StorageProviderTypeKey)) if providerType != ProviderS3.String() { - return clues.New("unsupported storage provider: " + providerType) + return S3Config{}, clues.New("unsupported storage provider: " + providerType) } // This is matching override values from config file. - if err := mustMatchConfig(kvs, s3Overrides(overrides)); err != nil { - return clues.Wrap(err, "verifying s3 configs in corso config file") + if err := mustMatchConfig(kvg, s3Overrides(overrides)); err != nil { + return S3Config{}, clues.Wrap(err, "verifying s3 configs in corso config file") } } } - s3Cfg = s3CredsFromStore(kvs, s3Cfg) + s3Cfg = s3CredsFromStore(kvg, s3Cfg) aws := credentials.GetAWS(overrides) if len(aws.AccessKey) <= 0 || len(aws.SecretKey) <= 0 { @@ -144,7 +145,7 @@ func (c S3Config) FetchConfigFromStore( } if err != nil { - return clues.Wrap(err, "validating aws credentials") + return S3Config{}, clues.Wrap(err, "validating aws credentials") } } @@ -163,7 +164,7 @@ func (c S3Config) FetchConfigFromStore( "false")), } - return nil + return s3Cfg, s3Cfg.validate() } var _ WriteConfigToStorer = S3Config{} @@ -242,7 +243,7 @@ var constToTomlKeyMap = map[string]string{ // mustMatchConfig compares the values of each key to their config file value in store. // If any value differs from the store value, an error is returned. // values in m that aren't stored in the config are ignored. -func mustMatchConfig(kvs KVStorer, m map[string]string) error { +func mustMatchConfig(kvg KVStoreGetter, m map[string]string) error { for k, v := range m { if len(v) == 0 { continue // empty variables will get caught by configuration validators, if necessary @@ -253,7 +254,7 @@ func mustMatchConfig(kvs KVStorer, m map[string]string) error { continue // m may declare values which aren't stored in the config file } - vv := cast.ToString(kvs.Get(tomlK)) + vv := cast.ToString(kvg.Get(tomlK)) if v != vv { return clues.New("value of " + k + " (" + v + ") does not match corso configuration value (" + vv + ")") } diff --git a/src/pkg/storage/s3_test.go b/src/pkg/storage/s3_test.go index 3a56fd090..116b5761c 100644 --- a/src/pkg/storage/s3_test.go +++ b/src/pkg/storage/s3_test.go @@ -66,9 +66,11 @@ func (suite *S3CfgSuite) TestStorage_S3Config() { in := goodS3Config s, err := NewStorage(ProviderS3, in) assert.NoError(t, err, clues.ToCore(err)) - out, err := s.S3Config() + sc, err := s.StorageConfig() 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) @@ -117,7 +119,7 @@ func (suite *S3CfgSuite) TestStorage_S3Config_invalidCases() { st, err := NewStorage(ProviderUnknown, goodS3Config) assert.NoError(t, err, clues.ToCore(err)) test.amend(st) - _, err = st.S3Config() + _, err = st.StorageConfig() assert.Error(t, err) }) } diff --git a/src/pkg/storage/storage.go b/src/pkg/storage/storage.go index c55169be2..b1d758b86 100644 --- a/src/pkg/storage/storage.go +++ b/src/pkg/storage/storage.go @@ -17,13 +17,9 @@ const ( ProviderFilesystem ProviderType = 2 // Filesystem ) -func StringToEnum(s string) StorageProvider { - switch s { - case ProviderS3.String(): - return ProviderS3 - } - - return ProviderUnknown +var StringToProviderType = map[string]ProviderType{ + ProviderUnknown.String(): ProviderUnknown, + ProviderS3.String(): ProviderS3, } // storage parsing errors @@ -34,6 +30,7 @@ var ( // Storage defines a storage provider, along with any configuration // required to set up or communicate with that provider. type Storage struct { + Provider ProviderType Provider ProviderType Config map[string]string // TODO: These are AWS S3 specific -> move these out @@ -44,6 +41,7 @@ type Storage struct { } // NewStorage aggregates all the supplied configurations into a single configuration. +func NewStorage(p ProviderType, cfgs ...common.StringConfigurer) (Storage, error) { func NewStorage(p ProviderType, cfgs ...common.StringConfigurer) (Storage, error) { cs, err := common.UnionStringConfigs(cfgs...) @@ -56,6 +54,7 @@ func NewStorage(p ProviderType, cfgs ...common.StringConfigurer) (Storage, error // NewStorageUsingRole supports specifying an AWS IAM role the storage provider // should assume. func NewStorageUsingRole( + p ProviderType, p ProviderType, roleARN string, sessionName string, @@ -92,7 +91,7 @@ func orEmptyString(v any) string { return v.(string) } -func (s Storage) GetStorageConfig() (StorageConfigurer, error) { +func (s Storage) StorageConfig() (Configurer, error) { switch s.Provider { case ProviderS3: return MakeS3ConfigFromMap(s.Config) @@ -101,32 +100,23 @@ func (s Storage) GetStorageConfig() (StorageConfigurer, error) { return nil, clues.New("unsupported storage provider: " + s.Provider.String()) } -func NewStorageConfig(provider string) (StorageConfigurer, error) { +func NewStorageConfig(provider ProviderType) (Configurer, error) { switch provider { - case ProviderS3.String(): + case ProviderS3: return S3Config{}, nil } - return nil, clues.New("unsupported storage provider: " + provider) + return nil, clues.New("unsupported storage provider: " + provider.String()) } -// Change it to just getter -type KVStorer interface { +type KVStoreGetter interface { Get(key string) any - Set(key string, value any) } type KVStoreSetter interface { Set(key string, value any) } -// Call it configurer if necessary. -type StorageConfigurer interface { - common.StringConfigurer - FetchConfigFromStorer - WriteConfigToStorer -} - type WriteConfigToStorer interface { WriteConfigToStore( kvs KVStoreSetter, @@ -135,9 +125,15 @@ type WriteConfigToStorer interface { type FetchConfigFromStorer interface { FetchConfigFromStore( - kv KVStorer, + kvg KVStoreGetter, readConfigFromStore bool, matchFromConfig bool, overrides map[string]string, - ) error + ) (Configurer, error) +} + +type Configurer interface { + common.StringConfigurer + FetchConfigFromStorer + WriteConfigToStorer }