Move S3 bucket flags to flags pkg (#4216)

<!-- PR description-->

Follow the pattern used elsewhere for declaring flag names & values. No logic change, just moving code. Added a unit test for flags.

---

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

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

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* #<issue>

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E

src/cli/flags/s3.go

@@ -0,0 +1,41 @@
+package flags
+
+import "github.com/spf13/cobra"
+
+// S3 bucket flags
+const (
+	BucketFN          = "bucket"
+	EndpointFN        = "endpoint"
+	PrefixFN          = "prefix"
+	DoNotUseTLSFN     = "disable-tls"
+	DoNotVerifyTLSFN  = "disable-tls-verification"
+	SucceedIfExistsFN = "succeed-if-exists"
+)
+
+// S3 bucket flag values
+var (
+	BucketFV          string
+	EndpointFV        string
+	PrefixFV          string
+	DoNotUseTLSFV     bool
+	DoNotVerifyTLSFV  bool
+	SucceedIfExistsFV bool
+)
+
+// S3 bucket flags
+func AddS3BucketFlags(cmd *cobra.Command) {
+	fs := cmd.Flags()
+
+	// Flags addition ordering should follow the order we want them to appear in help and docs:
+	// More generic and more frequently used flags take precedence.
+	fs.StringVar(&BucketFV, BucketFN, "", "Name of S3 bucket for repo. (required)")
+	fs.StringVar(&PrefixFV, PrefixFN, "", "Repo prefix within bucket.")
+	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"))
+}

src/cli/repo/s3.go

@@ -7,7 +7,6 @@ import (
 	"github.com/alcionai/clues"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
-	"github.com/spf13/pflag"
 
 	"github.com/alcionai/corso/src/cli/config"
 	"github.com/alcionai/corso/src/cli/flags"
@@ -20,41 +19,18 @@ import (
 	"github.com/alcionai/corso/src/pkg/storage"
 )
 
-// s3 bucket info from flags
-var (
-	succeedIfExists bool
-	bucket          string
-	endpoint        string
-	prefix          string
-	doNotUseTLS     bool
-	doNotVerifyTLS  bool
-)
-
-// s3 bucket flags
-const (
-	succeedIfExistsFN = "succeedIfExists"
-	bucketFN          = "bucket"
-	endpointFN        = "endpoint"
-	prefixFN          = "prefix"
-	doNotUseTLSFN     = "disable-tls"
-	doNotVerifyTLSFN  = "disable-tls-verification"
-)
-
 // called by repo.go to map subcommands to provider-specific handling.
 func addS3Commands(cmd *cobra.Command) *cobra.Command {
-	var (
+	var c *cobra.Command
-		c  *cobra.Command
-		fs *pflag.FlagSet
-	)
 
 	switch cmd.Use {
 	case initCommand:
 		init := s3InitCmd()
 		flags.AddRetentionConfigFlags(init)
-		c, fs = utils.AddCommand(cmd, init)
+		c, _ = utils.AddCommand(cmd, init)
 
 	case connectCommand:
-		c, fs = utils.AddCommand(cmd, s3ConnectCmd())
+		c, _ = utils.AddCommand(cmd, s3ConnectCmd())
 	}
 
 	c.Use = c.Use + " " + s3ProviderCommandUseSuffix
@@ -63,19 +39,7 @@ func addS3Commands(cmd *cobra.Command) *cobra.Command {
 	flags.AddAWSCredsFlags(c)
 	flags.AddAzureCredsFlags(c)
 	flags.AddCorsoPassphaseFlags(c)
-
+	flags.AddS3BucketFlags(c)
-	// Flags addition ordering should follow the order we want them to appear in help and docs:
-	// More generic and more frequently used flags take precedence.
-	fs.StringVar(&bucket, bucketFN, "", "Name of S3 bucket for repo. (required)")
-	fs.StringVar(&prefix, prefixFN, "", "Repo prefix within bucket.")
-	fs.StringVar(&endpoint, endpointFN, "", "S3 service endpoint.")
-	fs.BoolVar(&doNotUseTLS, doNotUseTLSFN, false, "Disable TLS (HTTPS)")
-	fs.BoolVar(&doNotVerifyTLS, 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(&succeedIfExists, "succeed-if-exists", false, "Exit with success if the repo has already been initialized.")
-	cobra.CheckErr(fs.MarkHidden("succeed-if-exists"))
 
 	return c
 }
@@ -173,7 +137,7 @@ func initS3Cmd(cmd *cobra.Command, args []string) error {
 		opt,
 		retentionOpts)
 	if err != nil {
-		if succeedIfExists && errors.Is(err, repository.ErrorRepoAlreadyExists) {
+		if flags.SucceedIfExistsFV && errors.Is(err, repository.ErrorRepoAlreadyExists) {
 			return nil
 		}
 
@@ -286,24 +250,24 @@ func PopulateS3Flags(flagset flags.PopulatedFlags) map[string]string {
 		s3Overrides[credentials.AWSSessionToken] = flags.AWSSessionTokenFV
 	}
 
-	if _, ok := flagset[bucketFN]; ok {
+	if _, ok := flagset[flags.BucketFN]; ok {
-		s3Overrides[storage.Bucket] = bucket
+		s3Overrides[storage.Bucket] = flags.BucketFV
 	}
 
-	if _, ok := flagset[prefixFN]; ok {
+	if _, ok := flagset[flags.PrefixFN]; ok {
-		s3Overrides[storage.Prefix] = prefix
+		s3Overrides[storage.Prefix] = flags.PrefixFV
 	}
 
-	if _, ok := flagset[doNotUseTLSFN]; ok {
+	if _, ok := flagset[flags.DoNotUseTLSFN]; ok {
-		s3Overrides[storage.DoNotUseTLS] = strconv.FormatBool(doNotUseTLS)
+		s3Overrides[storage.DoNotUseTLS] = strconv.FormatBool(flags.DoNotUseTLSFV)
 	}
 
-	if _, ok := flagset[doNotVerifyTLSFN]; ok {
+	if _, ok := flagset[flags.DoNotVerifyTLSFN]; ok {
-		s3Overrides[storage.DoNotVerifyTLS] = strconv.FormatBool(doNotVerifyTLS)
+		s3Overrides[storage.DoNotVerifyTLS] = strconv.FormatBool(flags.DoNotVerifyTLSFV)
 	}
 
-	if _, ok := flagset[endpointFN]; ok {
+	if _, ok := flagset[flags.EndpointFN]; ok {
-		s3Overrides[storage.Endpoint] = endpoint
+		s3Overrides[storage.Endpoint] = flags.EndpointFV
 	}
 
 	return s3Overrides

src/cli/utils/flags_test.go

@@ -91,3 +91,34 @@ func (suite *FlagUnitSuite) TestAddCorsoPassphraseFlags() {
 	err := cmd.Execute()
 	require.NoError(t, err, clues.ToCore(err))
 }
+
+func (suite *FlagUnitSuite) TestAddS3BucketFlags() {
+	t := suite.T()
+
+	cmd := &cobra.Command{
+		Use: "test",
+		Run: func(cmd *cobra.Command, args []string) {
+			assert.Equal(t, "bucket1", flags.BucketFV, flags.BucketFN)
+			assert.Equal(t, "endpoint1", flags.EndpointFV, flags.EndpointFN)
+			assert.Equal(t, "prefix1", flags.PrefixFV, flags.PrefixFN)
+			assert.Equal(t, true, flags.DoNotUseTLSFV, flags.DoNotUseTLSFN)
+			assert.Equal(t, true, flags.DoNotVerifyTLSFV, flags.DoNotVerifyTLSFN)
+			assert.Equal(t, true, flags.SucceedIfExistsFV, flags.SucceedIfExistsFN)
+		},
+	}
+
+	flags.AddS3BucketFlags(cmd)
+	// Test arg parsing for few args
+	cmd.SetArgs([]string{
+		"test",
+		"--" + flags.BucketFN, "bucket1",
+		"--" + flags.EndpointFN, "endpoint1",
+		"--" + flags.PrefixFN, "prefix1",
+		"--" + flags.DoNotUseTLSFN,
+		"--" + flags.DoNotVerifyTLSFN,
+		"--" + flags.SucceedIfExistsFN,
+	})
+
+	err := cmd.Execute()
+	require.NoError(t, err, clues.ToCore(err))
+}