aws/session: Add support for shared config (~/.aws/config) file #761

jasdel · 2016-07-18T20:55:53Z

Updates the SDK to take advantage of the new AWS_SDK_LOAD_CONFIG environment variable. When set to a truthy value, as defined in strconv.ParseBool, the SDK will load both shared config file (~~/.aws/config), and the shared credentials file (~~/.aws/credentials). The shared credentials file has precedence over the shared config.

Both config files share the same format. Both [name] and [profile name] are accepted as section names. The first, [name] will be looked for first before falling back to [profile name] per config file.

See the session package docs for more information.

Deprecates the session.New method since it cannot return an error when the session fails to load until a request is made. session.NewSession corrects this by returning an error when creating the session.

Fix #384

xibz · 2016-07-20T22:07:52Z

aws/session/env_config.go

+	return cfg
+}
+
+func setEnvValue(dst *string, keys []string) {


This was a little confusing to me by the name. I was thinking this was exporting to some environment variable. Perhaps getEnvValue(key []string) string may make more sense or something along those lines?

Good point that makes a lot more sense. I'll update the naming.

xibz · 2016-07-22T19:41:43Z

aws/session/env_config.go

+
+	// Require logical grouping of credentials
+	if len(cfg.Creds.AccessKeyID) == 0 || len(cfg.Creds.SecretAccessKey) == 0 {
+		cfg.Creds = credentials.Value{}


What is the provider name here? Do we care about empty ProviderName's?

xibz · 2016-07-22T20:29:36Z

Looks good with the few comments I have.

dlsniper · 2016-07-27T15:56:08Z

aws/session/session.go

+	return buildSession(envCfg, cfgs...)
+}
+
+func oldNewSession(cfgs ...*aws.Config) *Session {


I suggest renaming this to something more obvious like newSessionFallback or similar.

Thanks I'll take a look at refactoring these methods to see if I can create a better naming scheme for them, that fits in.

jasdel · 2016-07-27T19:26:16Z

Fixed loading environment variable for the shared config file, and updated New error handling to not panic, and be inline with how errors would of been reported when requests are made.

jasdel · 2016-07-29T00:54:42Z

awstesting/integration/customizations/s3/s3manager/integration_test.go

@@ -58,7 +58,7 @@ func setup() {

 // Delete the bucket
 func teardown() {
-	svc := s3.New(session.New())
+	svc := s3.New(session.NewFromSharedConfig())


Need to update integ tests for correct way to create session.

Adds opt-in support for loading the SDK's configuration from the shared config file (~/.aws/config) in addition to the current shared configuration file (~/.aws/credentials). This option can be enabled with the env var `AWS_SDK_LOAD_CONFIG` set to a truthy value, or creating a session with the `session.NewFromSharedConfig()` helper instead of `session.New()`. In addition to loading from the shared config file the SDK will also now load region from the shared credentials file when the share config is enabled. This changes also treats the syntax of both shared config and credentials as the same. See the session package for more information and examples. Adds shared config support for aws#384

jasdel · 2016-07-29T17:58:54Z

examples will be updated in another PR

xibz · 2016-07-29T20:57:12Z

aws/session/shared_config.go

+	// Trim files from the list that don't exist.
+	for _, f := range files {
+		if err := cfg.setFromIniFile(profile, f); err != nil {
+			if aerr, ok := err.(awserr.Error); ok && aerr.Code() == "SharedConfigProfileNotExist" {


SharedConfigProfileNotExist probably should be a const, since used in multiple places

Sounds good was thinking about converting these to typed errors for simplicity anyways, so this would work out well.

xibz · 2016-07-29T21:04:57Z

@jasdel - Looks pretty good! I have added few comments.

Updates the SDK to take advantage of the new `AWS_SDK_LOAD_CONFIG` environment variable. When set to a truthy value, as defined in strconv.ParseBool, the SDK will load both shared config file (~/.aws/config), and the shared credentials file (~/.aws/credentials). The shared credentials file has precedence over the shared config. Both config files share the same format. Both `[name]` and `[profile name]` are accepted as section names. The first, `[name]` will be looked for first before falling back to `[profile name]` per config file. See the [session](http://docs.aws.amazon.com/sdk-for-go/api/aws/session/) package docs for more information. Deprecates the `session.New` method since it cannot return an error when the session fails to load until a request is made. `session.NewSession` corrects this by returning an error when creating the session.

xibz · 2016-08-02T18:13:07Z

LGTM,

References: #384, #743, #761, #783, #784, #785

update dependencies and snowball test

jasdel added the Review Needed label Jul 18, 2016

jasdel assigned xibz Jul 18, 2016

jasdel force-pushed the feature/SharedConfig branch from e40c8ae to f111f84 Compare July 18, 2016 20:56

jasdel mentioned this pull request Jul 18, 2016

SDK config differs from AWS cli tool #384

Closed

jasdel force-pushed the feature/SharedConfig branch from f111f84 to c733c55 Compare July 18, 2016 23:53

xibz reviewed Jul 20, 2016
View reviewed changes

jasdel force-pushed the feature/SharedConfig branch from c733c55 to 0aec1ed Compare July 21, 2016 21:21

xibz reviewed Jul 22, 2016
View reviewed changes

jasdel force-pushed the feature/SharedConfig branch 4 times, most recently from 6f8b31c to a8f9089 Compare July 26, 2016 20:35

dlsniper reviewed Jul 27, 2016
View reviewed changes

jasdel force-pushed the feature/SharedConfig branch from f082cb0 to 2c03fa3 Compare July 29, 2016 00:38

jasdel reviewed Jul 29, 2016
View reviewed changes

jasdel added 2 commits July 28, 2016 22:05

awstesting/integration: Update integration tests for Shared Config

9231276

jasdel force-pushed the feature/SharedConfig branch 2 times, most recently from 092e1ea to 86ca8d9 Compare July 29, 2016 17:54

xibz reviewed Jul 29, 2016
View reviewed changes

jasdel force-pushed the feature/SharedConfig branch from 86ca8d9 to f23263f Compare August 1, 2016 22:58

jasdel removed the Review Needed label Aug 2, 2016

jasdel merged commit beae13f into aws:master Aug 2, 2016

jasdel deleted the feature/SharedConfig branch August 2, 2016 18:28

jasdel mentioned this pull request Aug 2, 2016

Update example code for Session with Shared Config #785

Merged

jasdel added a commit that referenced this pull request Aug 2, 2016

Tag release v1.3.0

eb0af7a

References: #384, #743, #761, #783, #784, #785

This was referenced Sep 5, 2016

Error fetching S3 data 99designs/iamy#18

Closed

Use the aws cli config 99designs/iamy#20

Merged

skotambkar pushed a commit to skotambkar/aws-sdk-go that referenced this pull request May 20, 2021

Merge pull request aws#761 from jasdel/integGlacier

0173a74

update dependencies and snowball test

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aws/session: Add support for shared config (~/.aws/config) file #761

aws/session: Add support for shared config (~/.aws/config) file #761

jasdel commented Jul 18, 2016 •

edited

Loading

xibz Jul 20, 2016

jasdel Jul 20, 2016

xibz Jul 22, 2016

xibz commented Jul 22, 2016

dlsniper Jul 27, 2016

jasdel Jul 27, 2016

jasdel commented Jul 27, 2016

jasdel Jul 29, 2016

jasdel commented Jul 29, 2016

xibz Jul 29, 2016

jasdel Jul 29, 2016

xibz commented Jul 29, 2016 •

edited

Loading

xibz commented Aug 2, 2016

aws/session: Add support for shared config (~/.aws/config) file #761

aws/session: Add support for shared config (~/.aws/config) file #761

Conversation

jasdel commented Jul 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xibz commented Jul 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasdel commented Jul 27, 2016

Choose a reason for hiding this comment

jasdel commented Jul 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xibz commented Jul 29, 2016 • edited Loading

xibz commented Aug 2, 2016

jasdel commented Jul 18, 2016 •

edited

Loading

xibz commented Jul 29, 2016 •

edited

Loading