Pass through errors up to the ultimate source of the call instead of …

…logging everywhere

cli/credentialslist.go

@@ -15,8 +15,8 @@ var listCredentialsCmd = &cobra.Command{
 	Use:   "list-credentials",
 	Short: "Resolves and lists all configured credentials",
 	Run: func(cmd *cobra.Command, args []string) {
-		if !configuration.Sources.ValidateConfiguration() {
-			log.Fatal("The sources section of the config file is invalid")
+		if err := configuration.Sources.ValidateConfiguration(); err != nil {
+			log.Fatalf("The sources section of the config file is invalid: %v", err)
 		}
 		allCredentials, err := configuration.Sources.Credentials()
 		if err != nil {

cli/sync.go

@@ -9,11 +9,11 @@ var syncCmd = &cobra.Command{
 	Use:   "sync",
 	Short: "Fetches credentials and syncs them to targets",
 	Run: func(cmd *cobra.Command, args []string) {
-		if !configuration.Sources.ValidateConfiguration() {
-			log.Fatal("The sources section of the config file is invalid")
+		if err := configuration.Sources.ValidateConfiguration(); err != nil {
+			log.Fatalf("The sources section of the config file is invalid: %v", err)
 		}
-		if !configuration.Targets.ValidateConfiguration() {
-			log.Fatal("The targets section of the config file is invalid")
+		if err := configuration.Targets.ValidateConfiguration(); err != nil {
+			log.Fatalf("The targets section of the config file is invalid: %v", err)
 		}
 		configuration.Sync()
 	},

cli/targetslist.go

@@ -11,8 +11,8 @@ var listTargetsCmd = &cobra.Command{
 	Use:   "list-targets",
 	Short: "Resolves and lists all configured targets",
 	Run: func(cmd *cobra.Command, args []string) {
-		if !configuration.Targets.ValidateConfiguration() {
-			log.Fatal("The targets section of the config file is invalid")
+		if err := configuration.Targets.ValidateConfiguration(); err != nil {
+			log.Fatalf("The targets section of the config file is invalid: %v", err)
 		}
 		for _, target := range configuration.Targets.AllTargets() {
 			fmt.Println(target.ToString())

cli/validate.go

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

credentials/credentials.go

@@ -4,17 +4,18 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/mitchellh/mapstructure"
 	log "github.com/sirupsen/logrus"
 )
 
 // Credentials defines the methods that can be called by all types of credentials
 type Credentials interface {
-	BaseValidate() bool
+	BaseValidate() error
 	GetID() string
 	ShouldSync(targetName string, targetTags map[string]string) bool
 	ToString(bool) string
-	Validate() bool
+	Validate() error
 }
 
 type targetTagsMatcher struct {
@@ -47,14 +48,14 @@ func (credBase *Base) BaseToString() string {
 }
 
 // BaseValidate verifies that the credentials fields common to all types of credentials contain valid values
-func (credBase *Base) BaseValidate() bool {
+func (credBase *Base) BaseValidate() error {
 	if credBase.ID == "" {
-		log.Errorf("Credentials (%s) has no defined ID", credBase.BaseToString())
+		return fmt.Errorf("Credentials (%s) has no defined ID", credBase.BaseToString())
 	}
 	if credBase.CredType == "" {
-		log.Errorf("Credentials (%s) has no type. This is probably a bug in the software", credBase.ID)
+		return fmt.Errorf("Credentials (%s) has no type. This is probably a bug in the software", credBase.ID)
 	}
-	return credBase.ID != "" && credBase.CredType != ""
+	return nil
 }
 
 // GetDescriptionOrID returns the description if it set, otherwise it returns the ID
@@ -145,8 +146,15 @@ func ParseSingleCredentials(credentialsMap map[string]interface{}) (Credentials,
 		return nil, errors.New("Unknown credentials type")
 	}
 	mapstructure.Decode(credentialsMap, credentials)
-	if !credentials.BaseValidate() || !credentials.Validate() {
-		return nil, errors.New("The following credentials failed to validate: \n	" + credentials.ToString(false))
+	var validationErrors error
+	if err := credentials.BaseValidate(); err != nil {
+		validationErrors = multierror.Append(validationErrors, err)
+	}
+	if err := credentials.Validate(); err != nil {
+		validationErrors = multierror.Append(validationErrors, err)
+	}
+	if validationErrors != nil {
+		return nil, fmt.Errorf("The following credentials failed to validate: %v -> %v", credentials.ToString(false), validationErrors)
 	}
 	return credentials, nil
 }

credentials/credentials_aws.go

@@ -3,8 +3,6 @@ package credentials
 import (
 	"fmt"
 	"strings"
-
-	log "github.com/sirupsen/logrus"
 )
 
 // AmazonWebServicesCredentials represents an access key id and a secret access key from AWS. A role to assume can also be defined
@@ -35,24 +33,21 @@ func (cred *AmazonWebServicesCredentials) ToString(showSensitive bool) string {
 
 // Validate verifies that the credentials is valid.
 // A AmazonWebServicesCredentials must define an access key and a secret access key
-func (cred *AmazonWebServicesCredentials) Validate() bool {
+func (cred *AmazonWebServicesCredentials) Validate() error {
 	if cred.AccessKey == "" && cred.SecretKey == "" && cred.Value != "" {
 		splitValue := strings.Split(cred.Value, ":")
 		if len(splitValue) != 2 {
-			log.Errorf("The credentials with ID %s has an invalid access_key:secret_key value: %s", cred.ID, cred.Value)
-			return false
+			return fmt.Errorf("The credentials with ID %s has an invalid access_key:secret_key value: %s", cred.ID, cred.Value)
 		}
 		cred.AccessKey = splitValue[0]
 		cred.SecretKey = splitValue[1]
 	}
 
 	if cred.AccessKey == "" {
-		log.Errorf("The credentials with ID %s does not define an access key", cred.ID)
-		return false
+		return fmt.Errorf("The credentials with ID %s does not define an access key", cred.ID)
 	}
 	if cred.SecretKey == "" {
-		log.Errorf("The credentials with ID %s does not define an secret key", cred.ID)
-		return false
+		return fmt.Errorf("The credentials with ID %s does not define an secret key", cred.ID)
 	}
-	return true
+	return nil
 }

credentials/credentials_secret.go

@@ -32,9 +32,9 @@ func (cred *SecretTextCredentials) ToString(showSensitive bool) string {
 
 // Validate verifies that the credentials is valid.
 // A SecretTextCredentials is always considered valid, as empty values are accepted.
-func (cred *SecretTextCredentials) Validate() bool {
+func (cred *SecretTextCredentials) Validate() error {
 	if cred.Secret == "" && cred.Value != "" {
 		cred.Secret = cred.Value
 	}
-	return true
+	return nil
 }

credentials/credentials_ssh.go

@@ -2,8 +2,6 @@ package credentials
 
 import (
 	"fmt"
-
-	log "github.com/sirupsen/logrus"
 )
 
 // SSHCredentials represents credentials composed of a private key, username and passphrase
@@ -40,10 +38,9 @@ func (cred *SSHCredentials) ToString(showSensitive bool) string {
 
 // Validate verifies that the credentials is valid.
 // A SSHCredentials must have a private key, the username and passphrase are optional
-func (cred *SSHCredentials) Validate() bool {
+func (cred *SSHCredentials) Validate() error {
 	if cred.PrivateKey == "" {
-		log.Errorf("The credentials with ID %s does not define a private key", cred.ID)
-		return false
+		return fmt.Errorf("The credentials with ID %s does not define a private key", cred.ID)
 	}
-	return true
+	return nil
 }

credentials/credentials_userpass.go

@@ -3,8 +3,6 @@ package credentials
 import (
 	"fmt"
 	"strings"
-
-	log "github.com/sirupsen/logrus"
 )
 
 // UsernamePasswordCredentials represents credentials composed of a username and a password
@@ -40,15 +38,14 @@ func (cred *UsernamePasswordCredentials) ToString(showSensitive bool) string {
 
 // Validate verifies that the credentials is valid.
 // A UsernamePasswordCredentials is always considered valid, as empty values are accepted.
-func (cred *UsernamePasswordCredentials) Validate() bool {
+func (cred *UsernamePasswordCredentials) Validate() error {
 	if cred.Username == "" && cred.Password == "" && cred.Value != "" {
 		splitValue := strings.Split(cred.Value, ":")
 		if len(splitValue) != 2 {
-			log.Errorf("The credentials with ID %s has an invalid username:password value: %s", cred.ID, cred.Value)
-			return false
+			return fmt.Errorf("The credentials with ID %s has an invalid username:password value: %s", cred.ID, cred.Value)
 		}
 		cred.Username = splitValue[0]
 		cred.Password = splitValue[1]
 	}
-	return true
+	return nil
 }

credentials/source_local.go

@@ -1,10 +1,9 @@
 package credentials
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
-
-	log "github.com/sirupsen/logrus"
 )
 
 // LocalSource represents local files containing credentials
@@ -23,12 +22,11 @@ func (source *LocalSource) Type() string {
 }
 
 // ValidateConfiguration verifies that the source's attributes are valid
-func (source *LocalSource) ValidateConfiguration() bool {
+func (source *LocalSource) ValidateConfiguration() error {
 	if _, err := os.Stat(source.File); os.IsNotExist(err) {
-		log.Errorf("%s does not exist\n", source.File)
-		return false
+		return fmt.Errorf("%s does not exist", source.File)
 	}
-	return true
+	return nil
 }
 
 func getCredentialsFromFile(fileName string) ([]Credentials, error) {

credentials/source_local_test.go

@@ -21,15 +21,15 @@ func TestGetCredentialsFromLocalSource(t *testing.T) {
 		File: filePath,
 	}
 	assert.Equal(t, "Local file", localSource.Type())
-	assert.False(t, localSource.ValidateConfiguration())
+	assert.Error(t, localSource.ValidateConfiguration())
 
 	ioutil.WriteFile(filePath, []byte(`test_cred:
   type: usernamepassword
   description: a credential
   username: user
   password: pass`), 0777)
 
-	assert.True(t, localSource.ValidateConfiguration())
+	assert.Nil(t, localSource.ValidateConfiguration())
 	credentials, err := localSource.Credentials()
 	expectedCred := NewUsernamePassword()
 	expectedCred.ID = "test_cred"

credentials/source_s3.go

@@ -1,26 +1,38 @@
 package credentials
 
 import (
+	"fmt"
 	"io/ioutil"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/credentials/stscreds"
 	"github.com/aws/aws-sdk-go/aws/session"
 	"github.com/aws/aws-sdk-go/service/s3"
-	log "github.com/sirupsen/logrus"
+	"github.com/aws/aws-sdk-go/service/s3/s3iface"
 )
 
 // AWSS3Source represents s3 objects containing credentials
 type AWSS3Source struct {
 	Bucket string
 	Key    string
+
+	client s3iface.S3API
+}
+
+func (source *AWSS3Source) getClient() s3iface.S3API {
+	if source.client == nil {
+		sess := session.Must(session.NewSessionWithOptions(session.Options{
+			SharedConfigState:       session.SharedConfigEnable,
+			AssumeRoleTokenProvider: stscreds.StdinTokenProvider,
+		}))
+		source.client = s3.New(sess)
+	}
+	return source.client
 }
 
 // Credentials extracts credentials from the source
 func (source *AWSS3Source) Credentials() ([]Credentials, error) {
-	sess := session.Must(session.NewSessionWithOptions(session.Options{
-		SharedConfigState: session.SharedConfigEnable,
-	}))
-	client := s3.New(sess)
+	client := source.getClient()
 
 	response, err := client.GetObject(&s3.GetObjectInput{
 		Bucket: aws.String(source.Bucket),
@@ -43,14 +55,12 @@ func (source *AWSS3Source) Type() string {
 }
 
 // ValidateConfiguration verifies that the source's attributes are valid
-func (source *AWSS3Source) ValidateConfiguration() bool {
+func (source *AWSS3Source) ValidateConfiguration() error {
 	if source.Bucket == "" {
-		log.Errorf("S3 sources must define a bucket")
-		return false
+		return fmt.Errorf("S3 sources must define a bucket")
 	}
 	if source.Key == "" {
-		log.Errorf("S3 sources must define a key")
-		return false
+		return fmt.Errorf("S3 sources must define a key")
 	}
-	return true
+	return nil
 }