Add support in TF file configuration for assuming roles via profiles defined in ~/.aws/config

[mikemoate]

This is simply a port of hashicorp/terraform#11734 to the new terraform-provider-aws repo. (the original PR won't be merged since the code it relates to has been moved to this repo).

This should address #1184.

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. (I certainly saw repeated prompts for MFA when testing adding this)

I can't see much in the way of test code in config_test.go so am not immediately sure where and how to test this successfuly.

Original work by @craiglink in hashicorp/terraform#11734.

[mikemoate]

After merging this PR, it seems that a rebuild/re-release of https://github.com/hashicorp/terraform and https://github.com/terraform-providers/terraform-provider-terraform will also be required to get this working consistently in all cases. Without this remote state backend and the remote state data source do not have the correct behaviour.

[mikemoate]

@Ninir what is needed to progress this PR, is there anything I can do? I'm keen to see this merged rather than forgotten.

I'm opening a new issue that I could also attempt a fix for, but I'd like to see progress on merging this before doing so.

[Ninir]

Hi @mikemoate

Will try to check this asap: don't know much about this specific part, but with some time I should be able to get on this ;)

[mikemoate]

Hi @Ninir it's been two more weeks and no activity on this PR, can this get prioritised please (it's not like other PR's aren't being reviewed and merged)?

Is there anyone else that can look at this?

[Ninir]

Hi @mikemoate

Sorry for the silence, 'had personal matters to manage 😅
Will defer to @catsby or @radeksimko on this one if you don't mind.

[wraithm]

@catsby and @radeksimko, I would love to get this PR merged as well. It is annoying that the aws provider doesn't support assume role profiles. What's the status of this PR?

[mikemoate]

I've rebased the branch against latest master, I'm not sure what else I can do to move this along, but would very much like to see it merged!

If you want to build with this support locally, you'll need to build this provider, the terraform-provider-terraform provider and terraform itself (since the last two directly pull in code from the AWS provider, making it somewhat less of a 'plugin').

https://gist.github.com/mikemoate/99776bee1c0b161f5bc2db207c771727 should cover all the needed steps on macOS (and has been tested by myself and another team member) but I'm far from an expert with govendor.

[mdunnio]

@catsby and @radeksimko, This would be great to have sooner rather than later. @mikemoate, I appreciate you trying to move this along!

[mikemoate]

I'm not sure how best to ensure that new releases of terraform-provider-terraform and terraform itself are made after merging this PR?

terraform itself doesn't seem to have vendored a specific version of terraform-provider-aws, so a simple update/refresh of the vendored dependency is needed. terraform-provider-terraform is sadly pinned to terraform-provider-aws@v0.1.4 which is somewhat behind the current...

Without these releases, profiles won't work in all places, which will no doubt lead to further end-user frustration.

[lorengordon]

@mikemoate I just built a binary for Windows (thanks for that gist!), but unfortunately I'm getting the error, No valid credential sources found for AWS Provider. So either I built it incorrectly, or I'm not setting up the provider correctly.

I set the profile in the provider as below, and also tried using the envs AWS_PROFILE and AWS_DEFAULT_PROFILE. I can use the profile with the aws cli just fine, so pretty sure it's not a problem with the aws config/credential files.

Any ideas?

provider "aws" {
  profile = "<myprofile>"
}

[lorengordon]

Realized that while I built the terraform binary for Windows, I probably need to also build the plugins for Windows. Looking into that now. Pointers welcome...

Edit: And I didn't checkout the correct branch when making the terraform binary. 🤦

[lorengordon]

Eef. Not seeing any instructions how to build plugins cross-platforms. No make on windows, and the XC_OS env (that works for terraform) doesn't appear to work for the plugins.

Anyway, I just copied my project to a throwaway linux box where I compiled everything, and the profile option does indeed work quite nicely. Awesome!

[mikemoate]

@lorengordon I can't offer much guidance for windows I'm afraid, though all of the go and git steps should work ok (you'll just need to replace the unix style file and directory operations).

My original gist also neglected to checkout the correct branch :-S but I think had been fixed before you looked at it.

[lorengordon]

@mikemoate No worries. Yes, I can setup the repos locally following your gist, no problem. I got stuck on the lack of make for Windows. Then thought I could just run go install directly, but that appears to run into this issue with go on Windows. Would be nice if the cross-platform build instructions for plugins were posted somewhere (as they are for terraform itself).

[lorengordon]

Alright, I cribbed from the terraform Makefile and build.sh script, and I was able to use gox to build cross-platform binaries for Windows. Happy to confirm this patch works fine on Windows also.

[mikemoate]

@radeksimko I see your comments on #1590 about this PR needing "some tests & more thorough review". I'd love to get that more thorough review soon :-)

I'd be happy to write some tests but would like some guidance on the best approach to take.

[cornfeedhobo]

Yeah, I would be interested in how one would add tests to this. Either way, I'd love to see it merged soon please.

[nelg]

+1

[lorengordon]

I wonder if this patch would get more attention if it were a new PR. Seems like there is a lot of actvity on the repo, just not older PRs.

[mikemoate]

@Ninir @radeksimko @catsby This PR is now 2 months old and still hasn't been reviewed by a Terraform maintainer, yet newer PR's have been reviewed and merged. I appreciate this is a significant change to consider, but surely that merits more attention, not less...

[wraithm]

@Ninir @radeksimko @catsby, This PR simplifies this and brings terraform in line with the standard AWS CLI tool's way of doing authorization with roles. It would be really great to get this in.

[svenwltr]

After #1590 was closed, I thought this topic would get a bit momentum, but it looks like nobody wants to touch it (or the responsible person is in vacation).

Currently we need to patch every release and distribute the binaries in our team because of this, which get very tedious over time.

[cornfeedhobo]

@Ninir what is holding up this review?

[Ninir]

Hi @mikemoate & folks!

Sorry for the latency here, We all have been working on different stuff, but things are getting back to normal :)

So I've played a bit with a custom build based on this work, and it runs pretty smoothly! 😄
Just left a few nitpicks here, nothing critical 👍

Just a small question: given we have this configuration (taken from a related issue):

[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

do you think it would be possible to handle the assume role logic when the profile is set as an env var, i.e. using AWS_DEFAULT_PROFILE & AWS_PROFILE?

it works nicely for non-assumed roles, but not for assumed ones :)

If you think this is something doable here, let's do it and merge, otherwise this should not be a blocker :)

Thanks a lot to everyone of you for your patience: we are about to cross the finish line :)

[Ninir]

Nitpick: specified

[Ninir]

Nitpick: This else is not needed since the if returns

[Ninir]

LGTM, thanks for the work @mikemoate and patience folks!

A big thanks to @craiglink for the original work! 🚀 😄

Merging!

[jaymecd]

🎉 Congrats!

[mikemoate]

@Ninir as discussed in my comments above, it's great that a new version of this provider will include this, but we also need to force terraform itself and terraform-provider-terraform to release versions that are compiled against this code, how do we achieve that?

https://gist.github.com/mikemoate/99776bee1c0b161f5bc2db207c771727 helps to illustrate the scale of the challenge here, especially for terraform-provider-terraform that has very old dependencies!

[Ninir]

we also need to force terraform itself and terraform-provider-terraform to be release versions that are compiled against this code, how do we achieve that?

@mikemoate Not sure to get it right :/
what about waiting for 1.3.0 which should be released in a few days?

Still, you can use a given provider by using Terraform 0.10.X & doing:

1. build the AWS provider (which will place the resulting binary in $GOPATH/bin)
2. go to your stack
3. terraform init -plugin-dir=$GOPATH/bin

Is there a reason you want to build terraform itself?

[mikemoate]

@Ninir both terraform and terraform-provider-terraform compile in source direct from this repo (including the source I changed), essentially violating the idea that the provider is a plugin :-S.

This seems to be used for remote state amongst other things.

Without new releases of both those that compile in this change, then the functionality only works in some cases, which isn't a great user experience...

[Ninir]

if I understand correctly: since the provider split, this repo (AWS provider) is really independent from the hashicorp/Terraform repo (the terraform core) regarding the code. Terraform just uses providers, i.e. binaries.

So you should be all ok using the official 0.10.8 Terraform binary for instance and this compiled provider using the command I wrote above.
What I mean is that this merge never goes in the hashicorp/terraform repo. If you already knew that, then forget about this 😄

If I don't understand correctly: can you provide more details? 😅

[mikemoate]

@Ninir So what I'm saying is that isn't true, both terraform and terraform-provider-terraform are NOT independent from this repo, and compile in source from it.

Check the following links for proof:

https://github.com/terraform-providers/terraform-provider-terraform/blob/master/vendor/vendor.json#L2000
https://github.com/hashicorp/terraform/blob/master/vendor/vendor.json#L2050

So both these repos need to re-vendor their dependencies to include the latest changes from this repo, and then compile against that.

[Ninir]

@mikemoate Ok so first: I read terraform-provider-AWS rather than terraform-provider-terraform... my bad 🤷‍♂️

Are you confirming they need to be in-sync, or is it just a supposition?

Let me investigate & discuss here what this relates to :)

[svenwltr]

@mikemoate @Ninir The Terraform reuses some code from terraform-provider-aws for the S3 remote state (since this lives in Terraform and not in the provider). The provider itself (ie aws_ resources) are independent.

Also terraform-provider-terraform uses it for accessing the remote statefile.

Edit
That's my information from when I patched Terraform. I'll lookup the actual source code references.

Edit2 https://github.com/hashicorp/terraform/blob/d477d1f6d4d6d6a25d76febf5b8e852ef8b054f1/backend/remote-state/s3/backend.go#L13

Edit3
AFAIS terraform-provider-terraform just reuses the terraform backend code, which itself uses the terraform-providerterraform`: https://github.com/terraform-providers/terraform-provider-terraform/blob/master/terraform/data_source_state.go#L8

[mikemoate]

@Ninir Oh I'm definitely confirming they need to be in-sync! :-) We use S3 based remote state and profile role assumption didn't work for this until both these were recompiled locally with update dependencies.

As @svenwltr notes terraform-provider-terraform pulls in the terraform repos backend code, which then also brings in the code from this provider (which is why it's a vendored dependency for that provider too).

It looks like terraform-provider-terraform is being removed for terraform 0.11.x onward, but I suggest we still fix it for any future releases of 0.10.x.

[svenwltr]

Hmm ... it sound like this PR only uses the profile from the Terraform config, but to me the change looks not like it gets the profile source didn't change, but only the underlying creation of the session. Therefore setting the profile via AWS_PROFILE should still work.

[trung]

@svenwltr indeed, there's a comment about it: #1608 (comment)

I'm going to open a new issue to request for env support. It may not be trivial as it seems better way of doing this is to utilize the SDK rather than patching in the current solution.

[drdamour]

what happens if the base account requires mfa and you run terraform plan

[mikemoate]

@Ninir any update on getting new releases of terraform and terraform-provider-terraform that include this? I'm worried that a closed PR is easily forgotten, perhaps we should raise issues to track those releases?

[Ninir]

Hi folks,

@trung @svenwltr I'm currently looking at how to refactor the logic to fully use the SDK: still theorical as of now, but will get into a PR very soon. This will include Env variables, conf, profiles,...

@drdamour MFA is not supported right now.

@mikemoate I opened a PR on the Core part: hashicorp/terraform#16661
This will probably be in sooner or later.

[choppedpork]

Hello,

I've been eagerly awaiting the 0.11.2 release to test this but unfortunately I'm still getting nowhere :(

My setup is as follows:

• no provider block specified in my modules or in my plan (this works fine with a .aws/credentials file using access / secret keys both with named profiles + setting the AWS_PROFILE environment variable as well as with a default profile)
• my role-assuming .aws/credentials:

[test]
aws_access_key_id = xxxxx
aws_secret_access_key = xxxxx
region = us-east-1

[default]
role_arn = arn:aws:iam::1234:role/Admin
source_profile = test
region = us-east-1

• remote state in my plan:

data "terraform_remote_state" "base" {
  backend = "s3"

  config {
    bucket = "xxx"
    key    = "terraform.tfstate"
    region = "us-east-1"
  }
}

• aws cli can retrieve the plan:

aws s3 cp s3://xxx/terraform.tfstate .
download: s3://xxx/terraform.tfstate to ./terraform.tfstate

• terraform cannot:

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.

data.template_file.user_data_without_ebs[0]: Refreshing state...
data.terraform_remote_state.base: Refreshing state...
data.template_file.user_data_without_ebs[1]: Refreshing state...

Error: Error refreshing state: 2 error(s) occurred:

* provider.aws: NoCredentialProviders: no valid providers in chain. Deprecated.
	For verbose messaging see aws.Config.CredentialsChainVerboseErrors
* data.terraform_remote_state.base: 1 error(s) occurred:

* data.terraform_remote_state.base: data.terraform_remote_state.base: error initializing backend: 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

I've tried adding a provider block to the plan but this didn't help much:

provider "aws" {
  shared_credentials_file = "~/.aws/credentials"
}

the output from terraform plan is identical.

I've tried a bunch of other things but the only one that made a difference was when I changed the profile name to something other than default and explicitly specified it in the provider block like so:

provider "aws" {
  shared_credentials_file = "~/.aws/credentials"
  profile                 = "test"
}

this is the terraform plan output for that run:

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.

data.terraform_remote_state.base: Refreshing state...
data.template_file.user_data_without_ebs[0]: Refreshing state...
data.template_file.user_data_without_ebs[1]: Refreshing state...
data.aws_availability_zones.azs: Refreshing state...
data.aws_availability_zones.azs: Refreshing state...
data.aws_iam_policy_document.instance-assume-role-policy: Refreshing state...
data.aws_ami.ami: Refreshing state...

Error: Error refreshing state: 1 error(s) occurred:

* data.terraform_remote_state.base: 1 error(s) occurred:

* data.terraform_remote_state.base: data.terraform_remote_state.base: error initializing backend: 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

[mikemoate]

We have the following terraform config, which I just tested with 0.11.2 and it works as expected (prior to this release we were building terraform from source):

// main.tf

terraform {
  backend "s3" {
    bucket  = "some-bucket-name"
    key     = "some-bucket-key/some-state.tfstate"
    profile = "some_profile"
    region  = "eu-west-2"
    acl     = "some-bucket-acl"
  }
}

provider "aws" {
  profile = "some_profile"
  region  = "eu-west-2"
}

// ~/.aws/config

[default]
output = json
region = eu-west-2

[profile some_profile]
role_arn = arn:aws:iam::123456789012:role/some_role
source_profile = default

// ~/.aws/credentials

[default]
aws_access_key_id=SOMEACCESSKEY
aws_secret_access_key=SomeSecretAccessKey

[choppedpork]

Thanks @mikemoate - I can confirm that once a named profile is explicitly specified in the backend block this all works fine including when using a 'default' profile - it just seems to need to be explicitly specified.

After trying a few more combinations, it looks like assuming roles works only works if the profiles are explicitly specified (even if it's just profile = "default") in the plan (had no luck using AWS_PROFILE for either the resources nor the state).

[mikemoate]

@choppedpork yeah #1184 discusses support for the AWS_PROFILE env var and I think there is a PR in progress.

[ghost]

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!