Return exit code 1 even if stop on error is enabled (#142)

* initial fix idea

* exit code 1 even when stop on error set to false

cli/credentialslist.go

@@ -15,17 +15,20 @@ var (
 var listCredentialsCmd = &cobra.Command{
 	Use:   "list-credentials",
 	Short: "Resolves and lists all configured credentials",
-	Run: func(cmd *cobra.Command, args []string) {
+	RunE: func(cmd *cobra.Command, args []string) error {
 		if err := configuration.Sources.ValidateConfiguration(); err != nil {
-			logger.Log.Fatalf("The sources section of the config file is invalid: %v", err)
+			logger.Log.Errorf("The sources section of the config file is invalid: %v", err)
+			return err
 		}
 		allCredentials, err := configuration.Sources.Credentials()
 		if err != nil {
-			panic(err)
+			logger.Log.Errorf("The credential extraction for all configured sources failed: %v", err)
+			return err
 		}
 		for _, credentials := range allCredentials {
 			fmt.Println(credentials.ToString(showSensitiveAttributes))
 		}
+		return nil
 	},
 }
 

cli/root.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/url"
-	"os"
 	"strings"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -34,6 +33,8 @@ var rootCmd = &cobra.Command{
 	syncs them to the given targets. This CLI is useful for
 	targets that do not support external credentials.
 	Support Jenkins only for now.`,
+	SilenceUsage:  true,
+	SilenceErrors: true,
 	PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
 		var (
 			configurationDict = map[string]interface{}{}
@@ -129,8 +130,6 @@ func init() {
 func Execute(commit string, date string, version string) {
 	rootCmd.Version = fmt.Sprintf("%s %s (%s)", version, commit, date)
 	if err := rootCmd.Execute(); err != nil {
-		fmt.Println(err)
-		logger.Log.Error(err)
-		os.Exit(1)
+		logger.Log.Fatal("Credential sync failed, the errors encountered are listed above.")
 	}
 }

cli/sync.go

@@ -8,15 +8,19 @@ import (
 var syncCmd = &cobra.Command{
 	Use:   "sync",
 	Short: "Fetches credentials and syncs them to targets",
-	Run: func(cmd *cobra.Command, args []string) {
+	RunE: func(cmd *cobra.Command, args []string) error {
 		if err := configuration.Sources.ValidateConfiguration(); err != nil {
-			logger.Log.Fatalf("The sources section of the config file is invalid: %v", err)
+			logger.Log.Errorf("The sources section of the config file is invalid: %v", err)
+			return err
 		}
 		if err := configuration.Targets.ValidateConfiguration(); err != nil {
-			logger.Log.Fatalf("The targets section of the config file is invalid: %v", err)
+			logger.Log.Errorf("The targets section of the config file is invalid: %v", err)
+			return err
 		}
 		if err := configuration.Sync(); err != nil {
-			logger.Log.Fatalf("The synchronization process failed: %v", err)
+			logger.Log.Errorf("The synchronization process failed: %v", err)
+			return err
 		}
+		return nil
 	},
 }

cli/targetslist.go

@@ -2,6 +2,7 @@ package cli
 
 import (
 	"fmt"
+
 	"github.com/coveooss/credentials-sync/logger"
 
 	"github.com/spf13/cobra"
@@ -10,12 +11,14 @@ import (
 var listTargetsCmd = &cobra.Command{
 	Use:   "list-targets",
 	Short: "Resolves and lists all configured targets",
-	Run: func(cmd *cobra.Command, args []string) {
+	RunE: func(cmd *cobra.Command, args []string) error {
 		if err := configuration.Targets.ValidateConfiguration(); err != nil {
-			logger.Log.Fatalf("The targets section of the config file is invalid: %v", err)
+			logger.Log.Errorf("The targets section of the config file is invalid: %v", err)
+			return err
 		}
 		for _, target := range configuration.Targets.AllTargets() {
 			fmt.Println(target.ToString())
 		}
+		return nil
 	},
 }

cli/validate.go

@@ -8,13 +8,16 @@ import (
 var validateCmd = &cobra.Command{
 	Use:   "validate",
 	Short: "Parses and validates the given configuration",
-	Run: func(cmd *cobra.Command, args []string) {
+	RunE: func(cmd *cobra.Command, args []string) error {
 		if err := configuration.Sources.ValidateConfiguration(); err != nil {
-			logger.Log.Fatalf("The sources section of the config file is invalid: %v", err)
+			logger.Log.Errorf("The sources section of the config file is invalid: %v", err)
+			return err
 		}
 		if err := configuration.Targets.ValidateConfiguration(); err != nil {
-			logger.Log.Fatalf("The targets section of the config file is invalid: %v", err)
+			logger.Log.Errorf("The targets section of the config file is invalid: %v", err)
+			return err
 		}
 		logger.Log.Info("The config file is valid!")
+		return nil
 	},
 }

sync/config.go

@@ -6,6 +6,7 @@ import (
 	"github.com/coveooss/credentials-sync/credentials"
 	"github.com/coveooss/credentials-sync/logger"
 	"github.com/coveooss/credentials-sync/targets"
+	"github.com/hashicorp/go-multierror"
 )
 
 // Configuration represents the parsed configuration file given to the application
@@ -50,12 +51,16 @@ func (config *Configuration) Sync() error {
 	for _, target := range allTargets {
 		go config.initTarget(target, creds, initChannel)
 	}
+	// We will use this to accumulate errors that happen if config.StopOnError is set to false
+	// the multierror.Error implements error so we use the interface to type the accumulator
+	var errorAccumulator error
 	for i := 0; i < len(allTargets); i++ {
 		initTarget := <-initChannel
 		if err, ok := initTarget.(error); ok {
 			if config.StopOnError {
 				return err
 			}
+			errorAccumulator = multierror.Append(errorAccumulator, err)
 			logger.Log.Error(err)
 		} else {
 			validTargets = append(validTargets, initTarget.(targets.Target))
@@ -72,7 +77,10 @@ func (config *Configuration) Sync() error {
 		// Check for errors. Errors are only passed back if StopOnError is true so this should always return
 		err := <-errorChannel
 		if err != nil {
-			return err
+			if config.StopOnError {
+				return err
+			}
+			errorAccumulator = multierror.Append(errorAccumulator, err)
 		}
 	}
 
@@ -81,7 +89,8 @@ func (config *Configuration) Sync() error {
 		parallelismChannel <- true
 	}
 
-	return nil
+	// This is either a nil, or a collection of past errors which we want to bubble up
+	return errorAccumulator
 }
 
 func (config *Configuration) initTarget(target targets.Target, creds []credentials.Credentials, channel chan interface{}) {
@@ -101,9 +110,11 @@ func (config *Configuration) initTarget(target targets.Target, creds []credentia
 }
 
 func (config *Configuration) syncCredentials(target targets.Target, credentialsList []credentials.Credentials, parallelismChannel chan bool, errorChannel chan error) {
-	var err error
+	// We will use this to accumulate errors that happen if config.StopOnError is set to false
+	// the multierror.Error implements error so we use the interface to type the accumulator
+	var errorAccumulator error
 	defer func() {
-		errorChannel <- err
+		errorChannel <- errorAccumulator
 		<-parallelismChannel
 	}()
 
@@ -114,12 +125,17 @@ func (config *Configuration) syncCredentials(target targets.Target, credentialsL
 		}
 	}
 
-	if err = config.UpdateListOfCredentials(target, filteredCredentials); err != nil {
-		return
+	if err := config.UpdateListOfCredentials(target, filteredCredentials); err != nil {
+		errorAccumulator = multierror.Append(errorAccumulator, err)
+		if config.StopOnError {
+			return
+		}
 	}
-	if err = config.DeleteListOfCredentials(target); err != nil {
-		return
+	if err := config.DeleteListOfCredentials(target); err != nil {
+		errorAccumulator = multierror.Append(errorAccumulator, err)
+		if config.StopOnError {
+			return
+		}
 	}
-
 	logger.Log.Infof("Finished sync to %s", target.GetName())
 }

sync/target.go

@@ -6,10 +6,14 @@ import (
 	"github.com/coveooss/credentials-sync/credentials"
 	"github.com/coveooss/credentials-sync/logger"
 	"github.com/coveooss/credentials-sync/targets"
+	"github.com/hashicorp/go-multierror"
 )
 
 // DeleteListOfCredentials deletes the configured list of credentials from the given target
 func (config *Configuration) DeleteListOfCredentials(target targets.Target) error {
+	// We will use this to accumulate errors that happen if config.StopOnError is set to false
+	// the multierror.Error implements error so we use the interface to type the accumulator
+	var errorAccumulator error
 	for _, id := range config.CredentialsToDelete {
 		if targets.HasCredential(target, id) {
 			logger.Log.Infof("[%s] Deleting %s", target.GetName(), id)
@@ -18,12 +22,13 @@ func (config *Configuration) DeleteListOfCredentials(target targets.Target) erro
 				if config.StopOnError {
 					return err
 				}
+				errorAccumulator = multierror.Append(errorAccumulator, err)
 				logger.Log.Error(err)
 			}
 		}
 	}
 
-	return nil
+	return errorAccumulator
 }
 
 // UpdateListOfCredentials syncs the given list of credentials to the given target
@@ -37,13 +42,17 @@ func (config *Configuration) UpdateListOfCredentials(target targets.Target, list
 		return false
 	}
 
+	// We will use this to accumulate errors that happen if config.StopOnError is set to false
+	// the multierror.Error implements error so we use the interface to type the accumulator
+	var errorAccumulator error
 	for _, credentials := range listOfCredentials {
 		logger.Log.Infof("[%s] Syncing %s", target.GetName(), credentials.GetTargetID())
 		if err := target.UpdateCredentials(credentials); err != nil {
 			err = fmt.Errorf("Failed to send credentials with ID %s to %s: %v", credentials.GetTargetID(), target.GetName(), err)
 			if config.StopOnError {
 				return err
 			}
+			errorAccumulator = multierror.Append(errorAccumulator, err)
 			logger.Log.Error(err)
 		}
 	}
@@ -61,6 +70,7 @@ func (config *Configuration) UpdateListOfCredentials(target targets.Target, list
 					if config.StopOnError {
 						return err
 					}
+					errorAccumulator = multierror.Append(errorAccumulator, err)
 					logger.Log.Error(err)
 				}
 			} else {
@@ -69,5 +79,5 @@ func (config *Configuration) UpdateListOfCredentials(target targets.Target, list
 		}
 	}
 
-	return nil
+	return errorAccumulator
 }