From 921542c0f542135f4a897bab5527e4e2a15e53a8 Mon Sep 17 00:00:00 2001 From: Abin Simon Date: Tue, 14 Mar 2023 10:04:34 +0530 Subject: [PATCH] Fix repo connect error when we don't have a config (#2771) Previously we were failing to connect to the repo if we did not have an existing config file present but could not run init to generate it (had already initialized the repo). Error: retrieving storage provider details: bucket is required to perform this command --- #### Does this PR need a docs update or release note? - [x] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [ ] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Issue(s) * Regression from https://github.com/alcionai/corso/pull/2534 #### Test Plan - [ ] :muscle: Manual - [ ] :zap: Unit test - [x] :green_heart: E2E --- CHANGELOG.md | 3 +++ src/cli/cli.go | 9 ++++++- src/cli/cli_test.go | 8 +++--- src/cli/repo/s3.go | 6 ++--- src/cli/repo/s3_e2e_test.go | 51 ++++++++++++++++++++++++++++--------- 5 files changed, 57 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cffd1c79e..cbab67736 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] (beta) +### Fixed +- Fix repo connect not working without a config file + ## [v0.5.0] (beta) - 2023-03-13 ### Added diff --git a/src/cli/cli.go b/src/cli/cli.go index c04afc616..051e859f6 100644 --- a/src/cli/cli.go +++ b/src/cli/cli.go @@ -68,7 +68,14 @@ func preRun(cc *cobra.Command, args []string) error { } if !slices.Contains(avoidTheseDescription, cc.Short) { - cfg, err := config.GetConfigRepoDetails(ctx, true, nil) + overrides := map[string]string{} + if cc.Short == "Connect to a S3 repository" { + // Get s3 overrides for connect. Ideally we also need this + // for init, but we don't reach this block for init. + overrides = repo.S3Overrides() + } + + cfg, err := config.GetConfigRepoDetails(ctx, true, overrides) if err != nil { log.Error("Error while getting config info to run command: ", cc.Use) return err diff --git a/src/cli/cli_test.go b/src/cli/cli_test.go index 2ef401bcb..bc593c80f 100644 --- a/src/cli/cli_test.go +++ b/src/cli/cli_test.go @@ -11,15 +11,15 @@ import ( "github.com/alcionai/corso/src/internal/tester" ) -type CLISuite struct { +type CLIUnitSuite struct { tester.Suite } -func TestCLISuite(t *testing.T) { - suite.Run(t, &CLISuite{Suite: tester.NewUnitSuite(t)}) +func TestCLIUnitSuite(t *testing.T) { + suite.Run(t, &CLIUnitSuite{Suite: tester.NewUnitSuite(t)}) } -func (suite *CLISuite) TestAddCommands_noPanics() { +func (suite *CLIUnitSuite) TestAddCommands_noPanics() { t := suite.T() test := &cobra.Command{ diff --git a/src/cli/repo/s3.go b/src/cli/repo/s3.go index 778daef4d..8381140bb 100644 --- a/src/cli/repo/s3.go +++ b/src/cli/repo/s3.go @@ -109,7 +109,7 @@ func initS3Cmd(cmd *cobra.Command, args []string) error { return nil } - cfg, err := config.GetConfigRepoDetails(ctx, false, s3Overrides()) + cfg, err := config.GetConfigRepoDetails(ctx, false, S3Overrides()) if err != nil { return Only(ctx, err) } @@ -177,7 +177,7 @@ func connectS3Cmd(cmd *cobra.Command, args []string) error { return nil } - cfg, err := config.GetConfigRepoDetails(ctx, true, s3Overrides()) + cfg, err := config.GetConfigRepoDetails(ctx, true, S3Overrides()) if err != nil { return Only(ctx, err) } @@ -208,7 +208,7 @@ func connectS3Cmd(cmd *cobra.Command, args []string) error { return nil } -func s3Overrides() map[string]string { +func S3Overrides() map[string]string { return map[string]string{ config.AccountProviderTypeKey: account.ProviderM365.String(), config.StorageProviderTypeKey: storage.ProviderS3.String(), diff --git a/src/cli/repo/s3_e2e_test.go b/src/cli/repo/s3_e2e_test.go index a26f9d4d9..cd1e8b123 100644 --- a/src/cli/repo/s3_e2e_test.go +++ b/src/cli/repo/s3_e2e_test.go @@ -1,6 +1,7 @@ package repo_test import ( + "os" "testing" "github.com/stretchr/testify/assert" @@ -30,16 +31,24 @@ func TestS3E2ESuite(t *testing.T) { func (suite *S3E2ESuite) TestInitS3Cmd() { table := []struct { - name string - bucketPrefix string + name string + bucketPrefix string + hasConfigFile bool }{ { - name: "NoPrefix", - bucketPrefix: "", + name: "NoPrefix", + bucketPrefix: "", + hasConfigFile: true, }, { - name: "S3Prefix", - bucketPrefix: "s3://", + name: "S3Prefix", + bucketPrefix: "s3://", + hasConfigFile: true, + }, + { + name: "NoConfigFile", + bucketPrefix: "", + hasConfigFile: false, }, } @@ -55,6 +64,11 @@ func (suite *S3E2ESuite) TestInitS3Cmd() { require.NoError(t, err) vpr, configFP := tester.MakeTempTestConfigClone(t, nil) + if !test.hasConfigFile { + // Ideally we could use `/dev/null`, but you need a + // toml file plus this works cross platform + os.Remove(configFP) + } ctx = config.SetViper(ctx, vpr) @@ -131,16 +145,24 @@ func (suite *S3E2ESuite) TestInitS3Cmd_missingBucket() { func (suite *S3E2ESuite) TestConnectS3Cmd() { table := []struct { - name string - bucketPrefix string + name string + bucketPrefix string + hasConfigFile bool }{ { - name: "NoPrefix", - bucketPrefix: "", + name: "NoPrefix", + bucketPrefix: "", + hasConfigFile: true, }, { - name: "S3Prefix", - bucketPrefix: "s3://", + name: "S3Prefix", + bucketPrefix: "s3://", + hasConfigFile: true, + }, + { + name: "NoConfigFile", + bucketPrefix: "", + hasConfigFile: false, }, } @@ -161,6 +183,11 @@ func (suite *S3E2ESuite) TestConnectS3Cmd() { tester.TestCfgPrefix: cfg.Prefix, } vpr, configFP := tester.MakeTempTestConfigClone(t, force) + if !test.hasConfigFile { + // Ideally we could use `/dev/null`, but you need a + // toml file plus this works cross platform + os.Remove(configFP) + } ctx = config.SetViper(ctx, vpr)