add bool format checks for string flags (#1072)

* add bool format checks for string flags

Adds a boolean-parseable validator for cli flags
that accept strings representing boolean values.

* fix ctx production in cli integration test

src/cli/backup/exchange.go

@@ -376,12 +376,11 @@ func exchangeDetailsCmd() *cobra.Command {
 
 // lists the history of backup operations
 func detailsExchangeCmd(cmd *cobra.Command, args []string) error {
-	ctx := cmd.Context()
-
 	if utils.HasNoFlagsAndShownHelp(cmd) {
 		return nil
 	}
 
+	ctx := cmd.Context()
 	opts := utils.ExchangeOpts{
 		Contacts:            contact,
 		ContactFolders:      contactFolder,
@@ -402,10 +401,6 @@ func detailsExchangeCmd(cmd *cobra.Command, args []string) error {
 		EventSubject:        eventSubject,
 	}
 
-	if err := utils.ValidateExchangeRestoreFlags(backupID, opts); err != nil {
-		return err
-	}
-
 	s, acct, err := config.GetStorageAndAccount(ctx, true, nil)
 	if err != nil {
 		return Only(ctx, err)
@@ -441,6 +436,10 @@ func runDetailsExchangeCmd(
 	backupID string,
 	opts utils.ExchangeOpts,
 ) (*details.Details, error) {
+	if err := utils.ValidateExchangeRestoreFlags(backupID, opts); err != nil {
+		return nil, err
+	}
+
 	d, _, err := r.BackupDetails(ctx, backupID)
 	if err != nil {
 		if errors.Is(err, kopia.ErrNotFound) {

src/cli/backup/exchange_test.go

@@ -250,22 +250,21 @@ func (suite *ExchangeSuite) TestExchangeBackupDetailsSelectorsBadBackupID() {
 	assert.Empty(t, output)
 }
 
-// TODO(ashmrtn): Uncomment these when the CLI validates flag input values.
+func (suite *ExchangeSuite) TestExchangeBackupDetailsSelectorsBadFormats() {
-//func (suite *ExchangeSuite) TestExchangeBackupDetailsSelectorsBadFormats() {
+	ctx, flush := tester.NewContext()
-// ctx, flush := tester.NewContext()
+	defer flush()
-// defer flush()
+
-//
+	for _, test := range testdata.BadExchangeOptionsFormats {
-//	for _, test := range testdata.BadExchangeOptionsFormats {
+		suite.T().Run(test.Name, func(t *testing.T) {
-//		suite.T().Run(test.Name, func(t *testing.T) {
+			output, err := runDetailsExchangeCmd(
-//			output, err := runDetailsExchangeCmd(
+				ctx,
-//				ctx,
+				test.BackupGetter,
-//				test.BackupGetter,
+				"backup-ID",
-//				"backup-ID",
+				test.Opts,
-//				test.Opts,
+			)
-//			)
+
-//			assert.Error(t, err)
+			assert.Error(t, err)
-//
+			assert.Empty(t, output)
-//			assert.Empty(t, output)
+		})
-//		})
+	}
-//	}
+}
-//}

src/cli/restore/exchange_integration_test.go

@@ -1,6 +1,7 @@
 package restore_test
 
 import (
+	"context"
 	"testing"
 
 	"github.com/spf13/viper"
@@ -176,3 +177,33 @@ func (suite *RestoreExchangeIntegrationSuite) TestExchangeRestoreCmd_badTimeFlag
 		})
 	}
 }
+
+func (suite *RestoreExchangeIntegrationSuite) TestExchangeRestoreCmd_badBoolFlags() {
+	for _, set := range backupDataSets {
+		if set != events {
+			suite.T().Skip()
+		}
+
+		suite.T().Run(set.String(), func(t *testing.T) {
+			ctx := config.SetViper(context.Background(), suite.vpr)
+			ctx, flush := tester.WithContext(ctx)
+			defer flush()
+
+			var timeFilter string
+			switch set {
+			case events:
+				timeFilter = "--event-recurs"
+			}
+
+			cmd := tester.StubRootCmd(
+				"restore", "exchange",
+				"--config-file", suite.cfgFP,
+				"--backup", string(suite.backupOps[set].Results.BackupID),
+				timeFilter, "wingbat")
+			cli.BuildCommandTree(cmd)
+
+			// run the command
+			require.Error(t, cmd.ExecuteContext(ctx))
+		})
+	}
+}

src/cli/utils/exchange.go

@@ -93,6 +93,10 @@ func ValidateExchangeRestoreFlags(backupID string, opts ExchangeOpts) error {
 		return errors.New("invalid time format for event-starts-before")
 	}
 
+	if !IsValidBool(opts.EventRecurs) {
+		return errors.New("invalid format for event-recurs")
+	}
+
 	return nil
 }
 

src/cli/utils/utils.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strconv"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
@@ -69,9 +70,19 @@ func IsValidTimeFormat(in string) bool {
 	}
 
 	_, err := common.ParseTime(in)
-	if err != nil {
+
-		return false
+	return err == nil
+}
+
+// IsValidTimeFormat returns true if the input is regonized as a
+// boolean.  Returns true if the input is zero valued, which
+// indicates that the flag was not called.
+func IsValidBool(in string) bool {
+	if len(in) == 0 {
+		return true
 	}
 
-	return true
+	_, err := strconv.ParseBool(in)
+
+	return err == nil
 }