Add env vars for assume_role block

[rberlind]

This PR adds three environment variables, AWS_ROLE_ARN, AWS_SESSION_NAME, and AWS_EXTERNAL_ID, that can be used with the assume_role block of the AWS provider. These are desired by some users who currently use CI/CD tools such as Jenkins to assume and pass roles to Terraform OSS but would like to use the same Terraform code with minimum changes with Terraform Enterprise (TFE) to which the CI/CD tools cannot pass a role. However, their use is not limited to TFE.

Users will be able to set these environment variables in their TFE workspaces. They will have to add an assume_role block to their AWS provider blocks in their existing code in order for the new environment variables to be used, but they will not have to provide any values for the attributes under the assume_role block. In other words, they will add the following to their AWS provider blocks:

 assume_role {
  }

I have verified that if a user does not set any of the three new environment variables, then the assume_role block will be ignored and other authentication credentials provided will be used by the AWS provider.

So, the addition of the new environment variables gives users the flexibility to use their current authentication methods when using Terraform OSS while using the assume_role block with TFE provided that they do set the environment variables.

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

Release note for CHANGELOG:

ENHANCEMENTS:
provider: Add `AWS_ROLE_ARN`, `AWS_SESSION_NAME`, and `AWS_EXTERNAL_ID` environment variables for use with the `assume_role` block

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

I did not run acceptance tests, but did do my own manual tests to validate that the environment variables were read by the AWS provider. Here is an example of DEBUG output from a terraform plan command for which I had set all three environment variables and had an empty assume_role block like the one above:

2019-06-13T14:51:56.822Z [DEBUG] plugin.terraform-provider-aws: 2019/06/13 14:51:56 [INFO] assume_role configuration set: (ARN: "arn:aws:iam::<REDACTED_ACCOUNT_ID>:role/roger-terraform-assumed-role", SessionID: "session-1234", ExternalID: "1234", Policy: "")

Additionally, I was able to do a terraform apply command that actually did provision an EC2 instance (using the aws_instance resource) even though the role associated with the EC2 instance running Terraform did not have IAM policy to do that. This proved that the EC2 instance running Terraform really had assumed the other role which did have that permission.

[rberlind]

Note that I did not add an environment variable for the policy attribute of the assume_role block because that would be a multi-line value and does not seem well-suited for an environment variable.

[apparentlymart]

Hi AWS provider team! 👋

Since we try to keep the authentication and configuration functionality consistent between the AWS provider and the S3 backend, could whoever eventually approves this change also send similar review to hashicorp/terraform#21718? I expect that any feedback would be common to both, so probably easier to have the same person review both.

[rberlind]

Thanks @apparentlymart

[bflad]

Hi @rberlind 👋 Thanks for submitting this.

For context as we look into this, are these environment variable names used in any AWS products currently? In my brief searching they are not listed in the AWS CLI documentation or AWS Go SDK documentation. If these are not currently used anywhere else, this might leave us in an awkward position of trying to create a standard where one does not exist, which means we may not want to use the AWS_ namespace. Any additional insight is appreciated, thanks!

It is probably worth mentioning that setting environment variables via Terraform variables may also be an existing solution in this situation: https://www.terraform.io/docs/configuration/variables.html#environment-variables

e.g. the following configuration

# untested Terraform 0.12 configuration example
variable "assume_role_arn" {
  type = string
}

provider "aws" {
  # ... potentially other configuration ...
  assume_role {
    role_arn = var.assume_role_arn
  }
}

Which can have the variable value injected via the TF_VARS_assume_role_arn environment variable.

As an aside: I'm also curious if it might make any sense for Terraform to have native support for reading environment variables as a built-in function:

# Quick design sketch, not implemented
provider "aws" {
  # ... potentially other configuration ...
  assume_role {
    role_arn = envvar("AWS_ROLE_ARN")
  }
}

This might more clearly show how environment variables (and their naming) can be used to control Terraform configuration values beyond the generic variable method that exists today.

Please let us know about the naming choices here and the potential above solutions so we can further discuss this. Thanks!

[rberlind]

Thanks @bflad.

No, I do not believe that the environment variable names I suggested are standard AWS environment variables. I had not realized that we prefer to reserve "AWS_" for those that are. I used "AWS_" because it seemed to make sense based on the names of the attributes. That being said, I don't necessarily feel that all "AWS_*" environment variables we might create have to match those used by AWS itself. But if that is the standard we're trying to adhere to, that is fine. What names would you suggest instead?

Using and setting Terraform variables the way you suggested by using TF_VAR_* environment variables is probably NOT what my customer would want since it would force more code changes than they would like to make at this time. More problematically, it might require them to always set values for the new variables, although if it worked with them set to "", then it might be ok.

With my approach, the only code the customer will have to add is the following:

In AWS provider blocks:

assume_role {
 }

For the corresponding PR for the S3 backend, they would not need to make any Terraform code changes at all.

So, adding the new environment variables will be much easier for them.

The idea of adding an envvar function to the AWS provider is a good one but would again require more code changes than my approach. Additionally, I suspect the envvar mechanism would have to be added to Terraform core and would only be added to Terraform 0.12. My customer is still using 0.11.x.

[rberlind]

I modified the new environment variables to start with "TF_AWS_ASSUME_ROLE_" as requested and removed the DefaultFuncs. The reading of the 3 environment varialbles, TF_AWS_ASSUME_ROLE_ARN, TF_AWS_ASSUME_ROLE_SESSION_NAME, and
TF_AWS_ASSUME_ROLE_EXTERNAL_ID, is now done with os.Getenv() starting at line 1032 of provider.go.

I updated index.html.markdown to describe the new env vars in a separate paragraph under the "Environment variables" section and updated the information about them in lines 285-294, removing the statements that the assume_role block nees to be present since that is no longer the case.

I tested this on my Mac after building the provider and putting it in ~/.terraform.d/plugins, running terraform init against a directory with Terraform code including an aws provider block without assume_role stanza, and setting the environment variables. I also set TF_LOG=TRACE and then ran terraform apply. I was able to see all three variables set in the trace logs and I was also able to provision an EC2 instance using the assumed role.

[bflad]

Just noting there is a new duplicate here: #9208

[binlab]

Any plans when it will be merged? Thanks!

[binlab]

@bflad @apparentlymart, any objections here why this PR not merged yet? Currently, we should use a workaround to provide assume_role block dynamically based on existing or not aTF_VARS_assume_role_arn environment variable, which little bit tricky and strange. So, I would be appreciated to see this useful functionality. Thank you!

[devopsrick]

This would be super helpful to get updated/merged... we currently run into numerous issues after transitioning to assumed roles that would be solved by this functionality.

[zhelding]

Pull request #21306 has significantly refactored the AWS Provider codebase. As a result, most PRs opened prior to the refactor now have merge conflicts that must be resolved before proceeding.

Specifically, PR #21306 relocated the code for all AWS resources and data sources from a single aws directory to a large number of separate directories in internal/service, each corresponding to a particular AWS service. This separation of code has also allowed for us to simplify the names of underlying functions -- while still avoiding namespace collisions.

We recognize that many pull requests have been open for some time without yet being addressed by our maintainers. Therefore, we want to make it clear that resolving these conflicts in no way affects the prioritization of a particular pull request. Once a pull request has been prioritized for review, the necessary changes will be made by a maintainer -- either directly or in collaboration with the pull request author.

For a more complete description of this refactor, including examples of how old filepaths and function names correspond to their new counterparts: please refer to issue #20000.

For a quick guide on how to amend your pull request to resolve the merge conflicts resulting from this refactor and bring it in line with our new code patterns: please refer to our Service Package Refactor Pull Request Guide.

[github-actions]

Marking this pull request as stale due to inactivity. This helps our maintainers find and focus on the active pull requests. If this pull request receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this pull request was automatically closed and you feel this pull request should be reopened, we encourage creating a new pull request linking back to this one for added context. Thank you!