terraform icon indicating copy to clipboard operation
terraform copied to clipboard

[WIP] - Add assume role with web identity support for the S3 backend

Open bschaatsbergen opened this issue 2 years ago • 5 comments

This pull request implements the feature requested in https://github.com/hashicorp/terraform/issues/31244.

Summary

Currently the S3 Backend does not support assuming a role with web identity. As we already support assuming a different role as part of the S3 backend this seems like a valid request.

Next to that I think that it's also right to have feature parity with the Terraform AWS provider as they added the OpenID Connect (OIDC) support too recently.

OIDC allows GitHub Actions and other CI/CD pipelines to access resources in Amazon Web Services (AWS), without needing to store the AWS credentials as long-lived GitHub secrets. Security wise this seems like a huge win for those that assume a different role for the S3 backend.

A little heads-up

There seems to be no support yet for AssumeRoleWithWebIdentity in the awsbase go package we're using. As support for AssumeRoleWithWebIdentity through awsbase is only added to the awsbase/v2 package.

I've refactored a part of the S3 backend code to be compatible with the awsbase/v2 package.

Next to that, to separate the assume role and assume role through web identity configurations I've offloaded the existing attributes to 2 different configuration blocks. assume_role and assume_role_with_web_identity. For both a cleaner and simpler interface as well as to be inline with the existing configuration blocks coming from the Terraform AWS provder.

For example the below existing Terraform configuration

terraform {
  backend "s3" {
    region     = "us-west-2"
    bucket     = "example-bucket"
    key        = "example/state.tfstate"
    role_arn = "arn:aws:iam::837424938642:role/example-role"
    assume_role_duration_seconds = 600
  }
}

Becomes as following:

terraform {
  backend "s3" {
    region     = "us-west-2"
    bucket     = "example-bucket"
    key        = "example/state.tfstate"
    
    assume_role {
      role_arn = "arn:aws:iam::837424938642:role/example-role"
      duration_seconds = 600
    }
  }
}

Similar to the new assume_role block, a assume_role_with_web_identity block has been added to support the feature request.

terraform {
  backend "s3" {
    region     = "us-west-2"
    bucket     = "example-bucket"
    key        = "example/state.tfstate"
    
    assume_role_with_web_identity {
      role_arn = "arn:aws:iam::837424938642:role/example-role"
      web_identity_token_file = "/example/tokenfile"
      duration_seconds = 600
    }
  }
}

(Optional) backwards compatible - if we want no breaking changes

This doesn't have to be a breaking change, even though existing assume role related attributes are moved to a nested attribute and renamed. We could make the change backwards compatible, the old attributes can still be provided and override the assume_role block and a deprecation warning is shown to the user.

Implementation

Due to the S3 backend code being fairly outdated, I was forced to refactor a portion of the code to be compatible with the awsbase/v2. Which isn't a bad thing as this helped to stay inline with the Terraform AWS provider, most of the code was years old and up for a refactor nevertheless.

I've created 2 new configuration blocks, assume_role and assume_role_with_web_identity_provider. I've also added a new parameter duration to the assume_role block to be inline with the assume_role_with_web_identity_provider block.

To be done

Here some points that I'm still working on:

(if we plan on having backwards compatible)

  • Add support for backwards compatibility
  • Deprecation warning if old attributes are set

Tests

To be added..

Example usage

To be added..

terraform {
  backend "s3" {
    key            = "25335/test.tfstate"
    bucket         = "tf-state-837424938642-eu-central-1"
    region         = "eu-central-1"
    encrypt        = true
    dynamodb_table = "tf-state-837424938642-eu-central-1"

    assume_role_with_web_identity {
      role_arn = "arn:aws:iam::837424938642:role/example-role"
      web_identity_token_file = "/example/tokenfile"
    }
  }
}

bschaatsbergen avatar Jun 19 '22 17:06 bschaatsbergen

CLA assistant check
All committers have signed the CLA.

hashicorp-cla avatar Jun 19 '22 17:06 hashicorp-cla

I'm going to wait for some feedback on this PR, whether it fits the roadmap, etc. before finalizing this.

bschaatsbergen avatar Jun 27 '22 20:06 bschaatsbergen

Can't wait <3

IrmantasMarozas avatar Sep 22 '22 08:09 IrmantasMarozas

@bschaatsbergen I don't know how long you are planning on waiting, but you might want to set the PR to 'Ready for review' to get it reviewed by one of the reviewers.

IskanderNovena avatar Oct 27 '22 13:10 IskanderNovena

@bschaatsbergen I don't know how long you are planning on waiting, but you might want to set the PR to 'Ready for review' to get it reviewed by one of the reviewers.

I'll have to dive into the PR again to sort a few things out. I'll see if I can spend some time on this over the weekend.

bschaatsbergen avatar Oct 27 '22 13:10 bschaatsbergen

@gdavison and I plan on revisiting this PR soon 👍🏼

bschaatsbergen avatar Jan 09 '23 20:01 bschaatsbergen

This capability would be really useful and could reduce a lot of complexity.

Any new information to share? Thanks, guys!

haakond avatar Feb 21 '23 07:02 haakond

When can we get this functionality in?

oxypwn avatar Apr 17 '23 20:04 oxypwn

Looking forward to this! Thanks @bschaatsbergen 🥇

thobalose avatar May 02 '23 13:05 thobalose

Hey @bschaatsbergen , thanks for taking the time to develop this, it will be incredibly handy for setting up secure s3 backends : )

Are there any updates on the state of this PR? From my understanding of the feature, it should be ready to review?

MattPalladino avatar May 16 '23 14:05 MattPalladino

Thanks for reaching out @MattPalladino, I've internally flagged this feature PR within Hashicorp and it's on the list of things to pick up - I believe that the S3 backend is part of the team that maintains the AWS Provider, which is currently busy working on their 5.0.0 release.

Tagging @breathingdust for visibility on this.

bschaatsbergen avatar May 16 '23 20:05 bschaatsbergen

Given AWS Provider 5.x is out for almost 2 months now, is there any news on this been released in next coming weeks/months? It would improve security and reduce operational complexity immensely.

alencar avatar Jul 11 '23 22:07 alencar

@breathingdust ☝🏼

bschaatsbergen avatar Jul 11 '23 22:07 bschaatsbergen

Hi @bschaatsbergen! We have just begun work on our S3 backend maintenance work in which this is included so we should have some movement on this very soon 🚀

breathingdust avatar Jul 12 '23 20:07 breathingdust

Ah that's amazing @breathingdust, thanks for the update 👍🏼

bschaatsbergen avatar Jul 12 '23 21:07 bschaatsbergen

Ah great to hear, @gdavison fixed! 👍🏼

  • Removed WIP from title
  • Moved from Draft to Ready for Review
  • Changed the base branch to s3/modernization

bschaatsbergen avatar Aug 23 '23 07:08 bschaatsbergen

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

github-actions[bot] avatar Aug 23 '23 14:08 github-actions[bot]

Thanks for your contribution, @bschaatsbergen!

The s3/modernization branch will be merged back onto main in the near future, targeting release with Terraform 1.6.0.

jar-b avatar Aug 23 '23 14:08 jar-b

@jar-b I'm trying to test it with IRSA, using the docker build hashicorp/terraform:1.6.0-beta1. Unfortunately still receiving the same error as 1.5.0, where it's trying to use the node (EC2) IAM role instead of pod's role injected via service account:

╷
│ Error: No valid credential sources found
│ 
│ Please see https://www.terraform.io/docs/language/settings/backends/s3.html
│ for more information about providing credentials.
│ 
│ Error: failed to refresh cached credentials, operation error STS:
│ AssumeRole, https response error StatusCode: 403, RequestID:
│ 1bb41fc4-9cd0-4e05-9897-34ff7615786f, api error AccessDenied: User:
│ arn:aws:sts::REDACTED:assumed-role/REDACTED-eks-node-group-REDACTED
│ is not authorized to perform: sts:AssumeRole on resource:
│ arn:aws:iam::REDACTED:role/gitlab-runner-rule-REDACTED

Which is the same problem describe in https://github.com/hashicorp/go-getter/issues/313

AWS_ROLE_ARN and AWS_WEB_IDENTITY_TOKEN_FILE env vars have correctly injected (validated with aws cli). Is this case covered by this PR and available in 1.6.0-beta1?

When I try to use the explicit configurations block:

assume_role_with_web_identity {
  role_arn = "arn:aws:iam::837424938642:role/example-role"
  web_identity_token_file = "/example/tokenfile"
}

I've the following error:

╷
│ Error: Unsupported block type
│ 
│   on dev.tfbackend line 11:
│   11: assume_role_with_web_identity {
│ 
│ Blocks of type "assume_role_with_web_identity" are not expected here.
╵

manobi avatar Sep 01 '23 23:09 manobi

Hi @manobi - Thanks for the report! It appears the initial implementation in the 1.6 beta is not checking for the presence of the AWS_ROLE_ARN and AWS_WEB_IDENTITY_TOKEN_FILE. We will address this for the next beta release.

The explicit configuration should work with a minor adjustment in syntax (= { instead of {):

assume_role_with_web_identity = {
  role_arn = "arn:aws:iam::837424938642:role/example-role"
  web_identity_token_file = "/example/tokenfile"
}

Please let us know if you continue to see issues with an explicit backend configuration, and thanks again for the bug report.

jar-b avatar Sep 05 '23 15:09 jar-b

Thanks you @jar-b I've made some progress by changing to the syntax you recommended.

Looking forward for the #33803, as soon as it get merged I'll come back here to report my tests.

manobi avatar Sep 06 '23 10:09 manobi

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions[bot] avatar Dec 10 '23 02:12 github-actions[bot]