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

Document AWS_SDK_LOAD_CONFIG on s3 backend #21122

Merged
merged 1 commit into from
May 29, 2019
Merged

Conversation

fernandrone
Copy link
Contributor

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Changes proposed in this pull request:

  • Document AWS_SDK_LOAD_CONFIG requirement when setting the S3 Backend AWS profile through the AWS_PROFILE variable.

Fixes:

There are multiple (closed) issues about this with calls for documentation, such as:

I've run some tests and confirmed that this is an issue both when configuring the S3 backend and when configuring the AWS provider (see other PR hashicorp/terraform-provider-aws#8451). I can detail the results if required, but given the number of existing issues and the aws-sdk-go documentation I think this behavior is as "confirmed" as it gets.

Notes:

AWS_SDK_LOAD_CONFIG is a configuration specific to AWS Golang SDK (and JavaScript SDK as well -- and no other SDK, as far as I can tell).

Per the AWS documentation, this must be set when using the Go SDK, otherwise AWS_PROFILE is ignored:

By default NewSession only loads credentials from the shared credentials file (~/.aws/credentials). If the AWS_SDK_LOAD_CONFIG environment variable is set to a truthy value, the session is created from the configuration values from the shared configuration (~/.aws/config) and shared credentials (~/.aws/credentials) files. See Sessions with a Shared Configuration File for more information.

This variable is not particularly well documented and seems to be a source of confusion, being limited to only two SDKs (Boto does not have it and profiles work on the fly). Personally if there's approval from the maintainers I'd edit the code to act as if AWS_SDK_LOAD_CONFIG is set true, but documenting seems to be the next best thing.

@hashicorp-cla
Copy link

hashicorp-cla commented Apr 26, 2019

CLA assistant check
All committers have signed the CLA.

@apparentlymart
Copy link
Contributor

Thanks for working on this, @fbcbarbosa!

If this setting is indeed specific to the Go SDK then I suspect we'd want to do something to make it not required, because our goal with the AWS provider and S3 backend is to behave the same way as other AWS client software and not require any special configuration.

With that said, I'd like to hold on this PR for the moment and see if the AWS provider team agrees with this and then, if so, whether we can find a fix that would apply to both so that this strange extra setting isn't required.

Thanks again!

@fernandrone
Copy link
Contributor Author

fernandrone commented Apr 26, 2019

Alright, thanks @apparentlymart !

According to the aws golang sdk docs, it seems one should be able to override this using NewSessionWithOptions:

Sessions can be created using the method above that will only load the additional config if the AWS_SDK_LOAD_CONFIG environment variable is set. Alternatively you can explicitly create a Session with shared config enabled. To do this you can use NewSessionWithOptions to configure how the Session will be created. Using the NewSessionWithOptions with SharedConfigState set to SharedConfigEnable will create the session as if the AWS_SDK_LOAD_CONFIG environment variable was set.


EDIT: updated this part, my initial analysis was wrong.

The s3 backend uses hashicorp/aws-sdk-go-base. It actually defines the session.SharedConfigEnable options here: https://github.com/hashicorp/aws-sdk-go-base/blob/master/session.go#L60

However, it does so only if Config.Profile is set (https://github.com/hashicorp/aws-sdk-go-base/blob/master/session.go#L42). When setting AWS_PROFILE from the environment variable, Config.Profile is not set, and this setting is ignored.

So a possible fix is to configure the CLI to set Config.Profile when AWS_PROFILE is set:

			"profile": {
				Type:        schema.TypeString,
				Optional:    true,
				Description: "AWS profile name",
				Default:     "",
			},

To:

			"profile": {
				Type:        schema.TypeString,
				Optional:    true,
				Description: "AWS profile name",
				Default:     schema.EnvDefaultFunc("AWS_PROFILE", ""),,
			},

Maybe support AWS_DEFAULT_PROFILE as well, but I think it's deprecated.


Anyway, I would've proposed this change from the beginning but I worried this'd need a fair bit of testing, and may need to wait for the next major version release since this might be considered a breaking change (there might be some terraform scripts out there running in an environment with AWS_PROFILE set, but AWS_SDK_LOAD_CONFIG unset, so they're actually using - knowingly or not - the default ~/.aws/credentials instead).

Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

This issue has cropped up for many practitioners and I believe adding the documentation here is a great first step to raising awareness of this odd environment variable. We can discuss further enhancements with SharedConfigEnable in a later feature request.

@bflad bflad merged commit 3865b74 into hashicorp:master May 29, 2019
@fernandrone fernandrone deleted the patch-1 branch July 2, 2019 14:17
@ghost
Copy link

ghost commented Jul 24, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Jul 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants