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

Issue: 7192 - Add support for AWS shared config file #11734

Closed
wants to merge 1 commit into from

Conversation

craiglink
Copy link

Add support to AWS Provider shared profiles via ~/.aws/config file
The AWS SDK for Go added support for this file in v 1.3
This change enables shared profile use if the existing creds.Get()
code path fails.

    Add support to AWS Provider shared profiles via ~/.aws/config file
    The AWS SDK for Go added support for this file in v 1.3
    This change enables shared profile use if the existing creds.Get()
    code path fails.
@catsby
Copy link
Contributor

catsby commented Feb 7, 2017

Hey @craiglink - can you give an example provider configuration block that utilizes this?
Is it as simple as this:

provider "aws" {
  profile = "someprofile"
}

It looks like profile exists in aws/provider.go and it just wasn't being used in aws/config.go, is that correct?

@catsby catsby added waiting-response An issue/pull request is waiting for a response from the community provider/aws bug labels Feb 7, 2017
@craiglink
Copy link
Author

craiglink commented Feb 7, 2017

This pull request adds support for a feature which the AWS CLI and SDKs have supported for some time. Specifically it adds support for a second configuration file named ~/.aws/config which has a extended format differing from the standard credentials file which only supports key pairs . One feature enabled by this extended format is assuming roles based on other profiles defined in the config file using the role_arn and source_profile keys. This allows the source profile to be create by various means, key pairs, MFA tokes, and SAML providers. It also allows the roles to vary from user to user, but allows the Terraform scripts to reference them by the same logic name.

The existing Terraform implementation supports a profile, but only looks in the ~/.aws/credentials file or specified shared_credentials_file using the basic credential file format. The AWS Provider also exposes the ability to role switch, but forces all users to use the same role because it’s declared in the Terraform scripts. Abstracting this to the AWS SDK credential provider provides more flexibility. To enable the extended format, session.NewSessionWithOptions must be specified to enable the SharedConfigFile option. Additionally the SDK is enabled to determine the appropriate credentials to use via the profile name verses providing them in the AwsConfig structure.

#User with admin rights

[default]
aws_access_key_id = KEY
aws_secret_access_key = SECRET
region = us-west-2
s3 =
    signature_version = s3v4

[profile dev]
role_arn = arn:aws:iam::XXXXXXXX671:role/admin
source_profile = default
region = us-west-2

[profile test]
role_arn = arn:aws:iam::XXXXXXXX824:role/admin
source_profile = default
region = us-west-2
#User with limited  rights

[default]
aws_access_key_id = KEY
aws_secret_access_key = SECRET
region = us-west-2
s3 =
    signature_version = s3v4

[profile dev]
role_arn = arn:aws:iam::XXXXXXXX671:role/restricted-dev
source_profile = default
region = us-west-2

[profile test]
role_arn = arn:aws:iam::XXXXXXXX824:role/restricted-test
source_profile = default
region = us-west-2
provider "aws" {
	# this could actually be referencing different AWS roles based on the ~/.aws/config file
	profile = “test”
}

@craiglink
Copy link
Author

@catsby just wanted to make sure you saw I responded. I assume I don't have a way to remove the "waiting for response" tag

@catsby catsby removed the waiting-response An issue/pull request is waiting for a response from the community label Feb 13, 2017
@craiglink
Copy link
Author

any updates on getting this merged in or questions or concerns?

@geoff-kramer
Copy link

+1 for this feature, my exact use case

@craiglink
Copy link
Author

@catsby any updates on getting this merged in or questions or concerns?

@ashb
Copy link
Contributor

ashb commented May 3, 2017

This would likely fix #11270 and #7014 too.

@kushmansingh
Copy link

+1 for this. I don't think this will play nicely with using a profile with MFA. I believe you'd have to add this option AssumeRoleTokenProvider: stscreds.StdinTokenProvider when initializing the AWS session based on http://docs.aws.amazon.com/sdk-for-go/api/aws/session/.

@kushmansingh
Copy link

#9349 + this PR would provide full AWS config + MFA support but unfortunately it looks like the other PR is abandoned.

@ashb
Copy link
Contributor

ashb commented May 5, 2017

@kushmansingh This PR would let us define the MFA serial etc in ~/.aws/config (or credentials) and letting the aws-sdk handle the MFA caching -- the other PR you mention is only needed if we want TF to handle the MFA caching/use natively inside terraform.

Which is perhaps a decision that the Terraform authors/contribs need to make:

  • Should we let TF handle every possible AWS auth scenario; or
  • Should we handle the simple cases (env vars, direct in the .tf files) natively and just say "for anything else set up a profile in ~/.aws?

I'd be learning towards the second one as it means that anything that works with aws on the command line would work with TF - useful for things like SAML or MFA which are used in corporate envs.

@kushmansingh
Copy link

kushmansingh commented May 5, 2017

@ashb I like the second option better as well. The only problem is that aws-sdk-go requires you to set the AssumeRoleTokenProvider option when using MFA as If it is not set an error will be returned when creating the session as mentioned in the session docs.

To avoid adding yet another option to the tf AWS provider, my recommendation would be to use the AssumeRoleTokenProvider: stscreds.StdinTokenProvider option I mentioned earlier.

This is a short example to illustrate my point https://play.golang.org/p/ama7Q8KMVI

CC: @craiglink

@ashb
Copy link
Contributor

ashb commented Jun 7, 2017

To any maintainers of Terraform: I'd very much like to see this make it in, and am willing to do the work to get a PR updated and merged.

However I don't want to do that work (and continue to keep it updated) if it's not going to be merged. If you @-mention me and let me know that 1) this feature is desired, and 2) anything about this current implementation I'll do the work to rebase and fix an issues on this PR. (if the original author is not around/doesn't have time to do it themselves)

@nelg
Copy link

nelg commented Aug 21, 2017

When is this going to be merged?, it's one outstanding issue I'd really like to see fixed.

@nwalke
Copy link
Contributor

nwalke commented Aug 21, 2017

This probably needs to be moved to the AWS provider before it'll be merged.

@et304383
Copy link

OK so this is my first time diving into terraform. I fully expected it would work with a profile using MFA and a role_arn and source_profile value, like such:

[myprofile]
output = json
region = us-east-1
role_arn = arn:aws:iam::<assume role account number>:role/Role_Name
source_profile = <source profile>
mfa_serial = arn:aws:iam::<IAM account number>:mfa/<username>

Running this should work:

export AWS_PROFILE=myprofile && terraform plan

Unfortunately this does not:

Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

Error refreshing state: 1 error(s) occurred:

* provider.aws: No valid credential sources found for AWS Provider.
  Please see https://terraform.io/docs/providers/aws/index.html for more information on
  providing credentials for the AWS Provider

There are too many issues floating around complaining and or trying to address different parts of this overall issue.

Can the Hashicorp team please just get together and issue a single pull request to make all functionality behave like the CLI, including caching temporary credentials? Also, setting a role_arn in a tf file feels fundamentally wrong. People reference different roles.

keymon added a commit to keymon/aws_iam_multiaccount that referenced this pull request Oct 1, 2017
The current policy to assume the roles requires
to have STS credentials with MultiFactorAuthPresent for the current
credentials.

But in the current implementation you must run terraform using
credentials with the role `admin`, to be able to access the bucket
with the state and the operations in the root account.

When terraform tries to assume the role for the submodule, it does it
without asking for the MFA credentials, so assume role
fails.

The solution would be to execute the project with the root user
credentials, and then terraform assumes role for but using MFA:
 - Getting the state for S3, using assume role[1]
 - Configure root account, users

The problem is that MFA support is not implemented in terraform[2].
It would require changes in the code [3] to add logic to ask for
the MFA token:

   assumeRoleProvider.SerialNumber = aws.String("arn:awss:iam::246505230303:mfa/hector.rivas+aws.admin@keytwine.com")
   assumeRoleProviderleProvider.TokenProvider = stscreds.StdinTokenProvider

Leaving the code here for reference

[1] https://www.terraform.io/docs/backends/types/s3.html#role_arn
[2] hashicorp/terraform#11734
[3] https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/auth_helpers.go#L211-L227
@cornfeedhobo
Copy link

What is the status on this PR?

@apparentlymart
Copy link
Contributor

Hello @craiglink, and thanks for working on this!

As part of the the Terraform 0.10 release earlier this year, all of the Terraform providers were moved to their own repositories in the terraform-providers GitHub organization, and removed from the Terraform Core repository.

Unfortunately due to the fact that new issues and pull requests are being opened constantly, it was not possible for the various provider maintainers to merge all outstanding pull requests before this split, and there is no automatic way to migrate a pull request to a new repository.

As a result, this pull request can sadly no longer be applied as-is, and so I'm going to close it.

If you or someone else has the time and motivation to apply same changes to the aws provider repository and open a new PR there, the maintainers of that provider should be able to review and merge it.

Thanks again for working on this, and sorry it was not able to be merged before the provider repository changes.

@svenwltr
Copy link
Contributor

@cornfeedhobo @apparentlymart This PR already got ported to the new repo. Unfortunately it also doesn't get much attention: hashicorp/terraform-provider-aws#1608

mikemoate added a commit to mikemoate/terraform-provider-aws that referenced this pull request Nov 14, 2017
This is simply a port of hashicorp/terraform#11734 to the new terraform-provider-aws.

(the original PR won't be merged since the code it relates to has been moved to a new module).

This won't support assuming roles where an MFA device has been specified for the profile (in ~/.aws/config). The original PR has some discussion of this, however, Terraform seems to utilise multiple, simultaneous sessions with AWS which complicates the situation somewhat.
Ninir pushed a commit to hashicorp/terraform-provider-aws that referenced this pull request Nov 14, 2017
…defined in ~/.aws/config (#1608)

* Add support for assuming roles via profiles defined in ~/.aws/config

This is simply a port of hashicorp/terraform#11734 to the new terraform-provider-aws.

(the original PR won't be merged since the code it relates to has been moved to a new module).

This won't support assuming roles where an MFA device has been specified for the profile (in ~/.aws/config). The original PR has some discussion of this, however, Terraform seems to utilise multiple, simultaneous sessions with AWS which complicates the situation somewhat.

* Address nitpick comments

Also remove leftover debug prefix on log message.
@ghost
Copy link

ghost commented Apr 6, 2020

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 Apr 6, 2020
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.