Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 3 commits into from
Apr 11, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7cb2e21
initial fix idea
pballandras Apr 6, 2022
5c86b10
exit code 1 even when stop on error set to false
pballandras Apr 8, 2022
f2ac783
boris review
pballandras Apr 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions cli/credentialslist.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}

Expand Down
7 changes: 3 additions & 4 deletions cli/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"io/ioutil"
"net/url"
"os"
"strings"

"github.com/aws/aws-sdk-go/aws"
Expand Down Expand Up @@ -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,
Comment on lines +36 to +37
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes it so when returning an error from a command, we don't:

  1. Show the usage string
  2. We tell the command we'll handle ourselves the errors coming from child commands

PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
var (
configurationDict = map[string]interface{}{}
Expand Down Expand Up @@ -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.")
}
}
12 changes: 8 additions & 4 deletions cli/sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}
7 changes: 5 additions & 2 deletions cli/targetslist.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cli

import (
"fmt"

"github.com/coveooss/credentials-sync/logger"

"github.com/spf13/cobra"
Expand All @@ -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
},
}
9 changes: 6 additions & 3 deletions cli/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}
34 changes: 25 additions & 9 deletions sync/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gives us a struct multierror that makes it possible to aggregate errors and that pretty print them for us.
e.g.

// printing
print(aMultiErrorWith4Errors)
// output 
4 errors occurred:
        * lol une erreur ici
        * lol une erreur ici
        * lol une erreur ici
        * lol une erreur ici

)

// Configuration represents the parsed configuration file given to the application
Expand Down Expand Up @@ -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))
Expand All @@ -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)
}
}

Expand All @@ -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{}) {
Expand All @@ -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
}()

Expand All @@ -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())
}
14 changes: 12 additions & 2 deletions sync/target.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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)
}
}
Expand All @@ -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 {
Expand All @@ -69,5 +79,5 @@ func (config *Configuration) UpdateListOfCredentials(target targets.Target, list
}
}

return nil
return errorAccumulator
}