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 aws provider #8451

Merged
merged 1 commit into from
Jun 7, 2019

Conversation

fernandrone
Copy link
Contributor

@fernandrone fernandrone commented Apr 26, 2019

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#21122). 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.

@ghost ghost added provider Pertains to the provider itself, rather than any interaction with AWS. documentation Introduces or discusses updates to documentation. size/XS Managed by automation to categorize the size of a PR. labels Apr 26, 2019
@apparentlymart
Copy link
Contributor

I left a comment about this over in hashicorp/terraform#21122 but want to note it over here for better visibility:

If this environment variable is not normally required by other AWS-integrated tools like the AWS CLI then I think the ideal path here would be to find a way to make Terraform's AWS provider and S3 backend not require it either, since I think our goal is that any user with a properly-configured and functioning AWS CLI should be able to run Terraform without any special further configuration to get their credentials to apply.

I don't know if that ideal can actually be achieved within the limitations of the SDK, but perhaps we should investigate that further (if we didn't already) and consider documenting this additional awkward extra step as a last resort?

@fernandrone
Copy link
Contributor Author

fernandrone commented Apr 26, 2019

Alright, thanks @apparentlymart !

I answered at hashicorp/terraform#21122 but I'm mostly copying the answer here, since it's relevant to both scenarios:

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.


The aws terraform provider 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:

https://github.com/hashicorp/terraform/blob/364c3e70b7cf5c1e6c6e4aea570d32dcc9f5ecc9/backend/remote-state/s3/backend.go#L130

			"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).

@@ -101,6 +101,9 @@ provider "aws" {
}
```

If specifying the profile through the `AWS_PROFILE` environment variable, you
must also set `AWS_SDK_LOAD_CONFIG` to a truthy value, e.g. `AWS_SDK_LOAD_CONFIG=1`
Copy link
Contributor

Choose a reason for hiding this comment

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

The wording of "must" here is overly strong, since AWS_PROFILE can successfully be used today in situations without AWS_SDK_LOAD_CONFIG. For example, the usage of only the AWS_PROFILE environment variable, works when the credentials are in a credentials file and the profile does not have any advanced configurations, such as: https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-profiles.html

See also: #8779 (comment)

My recommendation would be to call out specific usages that require the additional environment variable, e.g.

Suggested change
must also set `AWS_SDK_LOAD_CONFIG` to a truthy value, e.g. `AWS_SDK_LOAD_CONFIG=1`
may also need to set `AWS_SDK_LOAD_CONFIG` to a truthy value (e.g. `AWS_SDK_LOAD_CONFIG=1`) for advanced AWS client configurations, such as profiles that use the `source_profile` or `role_arn` configurations.

@bflad bflad added the waiting-response Maintainers are waiting on response from community or contributor. label May 29, 2019
@bflad
Copy link
Contributor

bflad commented Jun 7, 2019

Merged with suggestion above. 👍

@ghost
Copy link

ghost commented Nov 3, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 3, 2019
@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Nov 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. size/XS Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS_PROVIDER not working, explicit 'provider=xx' works fine
3 participants