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.

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

restoring default credentials object #86

Merged
merged 1 commit into from Jun 11, 2016
Merged

restoring default credentials object #86

merged 1 commit into from Jun 11, 2016

Conversation

jfwarner
Copy link
Contributor

@jfwarner jfwarner commented Jun 8, 2016

Fix for #84

Restoring default credentials object to "a chain of credential providers to search for credentials in environment variables, shared credential file, and EC2 Instance Roles"

http://docs.aws.amazon.com/sdk-for-go/api/aws/Config.html#Credentials-field

Previously, the "Credentials" field was left nil if there was no "--profile" argument. Now the Credentials field will be left at the default.

…ers to search for credentials in environment variables, shared credential file, and EC2 Instance Roles"
@kahing
Copy link
Owner

kahing commented Jun 9, 2016

I was always under the assumption that the default value for a struct pointer is nil, is that not the case?

@jfwarner
Copy link
Contributor Author

jfwarner commented Jun 9, 2016

I am a Go novice, so I don't quite understand everything that is going on yet. But this documentation leads me to believe otherwise:
http://docs.aws.amazon.com/sdk-for-go/api/aws/Config.html#Credentials-field

I also tested this change, and it behaves exactly the way the above documentation describes.

@jfwarner
Copy link
Contributor Author

jfwarner commented Jun 9, 2016

Perhaps it is the case that a nil value induces the desired behavior in the SDK, as in:
if( field is nil) then { use default chain };

In either case, it appears to work.

@kahing
Copy link
Owner

kahing commented Jun 11, 2016

I found this difficult to believe because if --profile is not used, awsConfig.Credentials is nil before and after the patch.

I will accept this anyway because it makes the config handling more consistent with other overrides.

@kahing kahing merged commit f64c04f into kahing:master Jun 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants