Skip to content

Commit

Permalink
Fix config load env, and New func error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
jasdel committed Jul 27, 2016
1 parent a8f9089 commit f082cb0
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 49 deletions.
2 changes: 1 addition & 1 deletion aws/session/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ file for the shared config. If not set the file will be loaded from
$HOME/.aws/config on Linux/Unix based systems, and
%USERPROFILE%\.aws\config on Windows.
AWS_SHARED_CONFIG_FILE=$HOME/my_shared_config
AWS_CONFIG_FILE=$HOME/my_shared_config
Shared Config Fields
Expand Down
2 changes: 1 addition & 1 deletion aws/session/env_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ type envConfig struct {
// $HOME/.aws/config on Linux/Unix based systems, and
// %USERPROFILE%\.aws\config on Windows.
//
// AWS_SHARED_CONFIG_FILE=$HOME/my_shared_config
// AWS_CONFIG_FILE=$HOME/my_shared_config
SharedConfigFile string
}

Expand Down
32 changes: 22 additions & 10 deletions aws/session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package session

import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/client"
"github.com/aws/aws-sdk-go/aws/corehandlers"
"github.com/aws/aws-sdk-go/aws/credentials"
Expand Down Expand Up @@ -30,30 +31,41 @@ type Session struct {
//
// If the AWS_SDK_LOAD_CONFIG environment is set to a truthy value, the New
// method could now encounter an error when loading the configuration. When
// the environment variable is set New will panic if an error occurs. The Build
// methods should be used instead to avoid this panic, as they will return an
// error if one occurs.
// The environment variable is set, and an error occurs, New will return a
// session that will fail all requests reporting the error that occured while
// loading the session. Use the Build funcs to get the error when creating
// the session.
//
// If the AWS_SDK_LOAD_CONFIG environment variable is set to a truthy value
// the shared config file (~/.aws/config) will also be loaded, in addition to
// the shared credentials file (~/.aws/config). Values set in both the shared
// shared config, and shared credentials will be taken from the shared
// credentials file.
//
// Deprecated: Use Build instead. Build has the same functionality as New
// except an error is returned instead of panic.
// Deprecated: Use Build instead. Build funcs have the same functionality as
// New except an error can be returned when the func is called instead of
// waiting to receive an error until a request is made.
func New(cfgs ...*aws.Config) *Session {
// load initial config from environment
envCfg := loadEnvConfig()

if envCfg.EnableSharedConfig {
// Old session.New expected all errors to be discovered when
// a request is made, and would report the errors then. This
// needs to be mocked.
s, err := buildSession(envCfg, cfgs...)
if err != nil {
// TODO error message
panic(err)
// Old session.New expected all errors to be discovered when
// a request is made, and would report the errors then. This
// needs to be replicated if an error occurs while creating
// the session.
msg := "failed to create session with AWS_SDK_LOAD_CONFIG enabled. " +
"Use session.Build to handle errors occuring during session creation."

// Session creation failed, need to report the error and prevent
// any requests from succeeding.
s = &Session{Config: defaults.Config()}
s.Config.Logger.Log("ERROR:", msg, "Error:", err)
s.Handlers.Validate.PushBack(func(r *request.Request) {
r.Error = awserr.New("SharedConfigLoadError", msg, err)
})
}
return s
}
Expand Down
91 changes: 54 additions & 37 deletions aws/session/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ import (
"github.com/stretchr/testify/assert"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/credentials"
"github.com/aws/aws-sdk-go/aws/defaults"
"github.com/aws/aws-sdk-go/service/s3"
)

func TestNewDefaultSession(t *testing.T) {
env := stashEnv()
defer popEnv(env)
os.Setenv("AWS_SHARED_CONFIG_FILE", "file_not_exists")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", "file_not_exists")
oldEnv := initSessionTestEnv()
defer popEnv(oldEnv)

s := New(&aws.Config{Region: aws.String("region")})

Expand All @@ -30,8 +30,8 @@ func TestNewDefaultSession(t *testing.T) {
}

func TestNew_WithCustomCreds(t *testing.T) {
env := stashEnv()
defer popEnv(env)
oldEnv := initSessionTestEnv()
defer popEnv(oldEnv)

customCreds := credentials.NewStaticCredentials("AKID", "SECRET", "TOKEN")
s := New(&aws.Config{Credentials: customCreds})
Expand All @@ -40,8 +40,8 @@ func TestNew_WithCustomCreds(t *testing.T) {
}

func TestSessionCopy(t *testing.T) {
env := stashEnv()
defer popEnv(env)
oldEnv := initSessionTestEnv()
defer popEnv(oldEnv)

os.Setenv("AWS_REGION", "orig_region")

Expand All @@ -67,10 +67,8 @@ func TestSessionClientConfig(t *testing.T) {
}

func TestBuildFromProfile(t *testing.T) {
env := stashEnv()
defer popEnv(env)
os.Setenv("AWS_SHARED_CONFIG_FILE", "file_not_exists")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", "file_not_exists")
oldEnv := initSessionTestEnv()
defer popEnv(oldEnv)

os.Setenv("AWS_SHARED_CREDENTIALS_FILE", testConfigFilename)
os.Setenv("AWS_PROFILE", "other_profile")
Expand All @@ -89,10 +87,8 @@ func TestBuildFromProfile(t *testing.T) {
}

func TestBuildFromProfile_WithSharedConfig(t *testing.T) {
env := stashEnv()
defer popEnv(env)
os.Setenv("AWS_SHARED_CONFIG_FILE", "file_not_exists")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", "file_not_exists")
oldEnv := initSessionTestEnv()
defer popEnv(oldEnv)

os.Setenv("AWS_SDK_LOAD_CONFIG", "1")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", testConfigFilename)
Expand All @@ -112,10 +108,8 @@ func TestBuildFromProfile_WithSharedConfig(t *testing.T) {
}

func TestBuildFromSharedConfig(t *testing.T) {
env := stashEnv()
defer popEnv(env)
os.Setenv("AWS_SHARED_CONFIG_FILE", "file_not_exists")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", "file_not_exists")
oldEnv := initSessionTestEnv()
defer popEnv(oldEnv)

os.Setenv("AWS_SDK_LOAD_CONFIG", "0")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", testConfigFilename)
Expand All @@ -137,10 +131,8 @@ func TestBuildFromSharedConfig(t *testing.T) {
func TestSessionAssumeRole_DisableSharedConfig(t *testing.T) {
// Backwards compatibility with Shared config disabled
// assume role should not be built into the config.
env := stashEnv()
defer popEnv(env)
os.Setenv("AWS_SHARED_CONFIG_FILE", "file_not_exists")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", "file_not_exists")
oldEnv := initSessionTestEnv()
defer popEnv(oldEnv)

os.Setenv("AWS_SDK_LOAD_CONFIG", "0")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", testConfigFilename)
Expand All @@ -157,10 +149,8 @@ func TestSessionAssumeRole_DisableSharedConfig(t *testing.T) {
}

func TestSesisonAssumeRole(t *testing.T) {
env := stashEnv()
defer popEnv(env)
os.Setenv("AWS_SHARED_CONFIG_FILE", "file_not_exists")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", "file_not_exists")
oldEnv := initSessionTestEnv()
defer popEnv(oldEnv)

os.Setenv("AWS_REGION", "us-east-1")
os.Setenv("AWS_SDK_LOAD_CONFIG", "1")
Expand Down Expand Up @@ -203,10 +193,8 @@ func TestSesisonAssumeRole(t *testing.T) {
func TestSessionAssumeRole_InvalidSourceProfile(t *testing.T) {
// Backwards compatibility with Shared config disabled
// assume role should not be built into the config.
env := stashEnv()
defer popEnv(env)
os.Setenv("AWS_SHARED_CONFIG_FILE", "file_not_exists")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", "file_not_exists")
oldEnv := initSessionTestEnv()
defer popEnv(oldEnv)

os.Setenv("AWS_SDK_LOAD_CONFIG", "1")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", testConfigFilename)
Expand Down Expand Up @@ -260,7 +248,7 @@ func TestBuildFromSharedConfigProfile(t *testing.T) {
InEnvs: map[string]string{
"AWS_SDK_LOAD_CONFIG": "0",
"AWS_SHARED_CREDENTIALS_FILE": testConfigFilename,
"AWS_SHARED_CONFIG_FILE": testConfigOtherFilename,
"AWS_CONFIG_FILE": testConfigOtherFilename,
"AWS_PROFILE": "shared_profile",
},
InProfile: "config_file_load_order",
Expand All @@ -274,10 +262,8 @@ func TestBuildFromSharedConfigProfile(t *testing.T) {
}

for _, c := range cases {
env := stashEnv()
defer popEnv(env)
os.Setenv("AWS_SHARED_CONFIG_FILE", "file_not_exists")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", "file_not_exists")
oldEnv := initSessionTestEnv()
defer popEnv(oldEnv)

for k, v := range c.InEnvs {
os.Setenv(k, v)
Expand All @@ -295,3 +281,34 @@ func TestBuildFromSharedConfigProfile(t *testing.T) {
assert.Contains(t, creds.ProviderName, c.OutCreds.ProviderName)
}
}

func TestNew_WithSessionLoadError(t *testing.T) {
oldEnv := initSessionTestEnv()
defer popEnv(oldEnv)

os.Setenv("AWS_SDK_LOAD_CONFIG", "1")
os.Setenv("AWS_CONFIG_FILE", testConfigFilename)
os.Setenv("AWS_PROFILE", "assume_role_invalid_source_profile")

s := New()

assert.NotNil(t, s)

svc := s3.New(s)
_, err := svc.ListBuckets(&s3.ListBucketsInput{})

assert.Error(t, err)
aerr := err.(awserr.Error)

assert.Equal(t, "SharedConfigLoadError", aerr.Code())
assert.Contains(t, aerr.Message(), "failed to create session")
assert.Contains(t, aerr.OrigErr().Error(), "SharedConfigAssumeRoleError")
}

func initSessionTestEnv() (oldEnv []string) {
oldEnv = stashEnv()
os.Setenv("AWS_CONFIG_FILE", "file_not_exists")
os.Setenv("AWS_SHARED_CREDENTIALS_FILE", "file_not_exists")

return oldEnv
}

0 comments on commit f082cb0

Please sign in to comment.