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

Add support for passing region information stored in vault backend to AWS Config #832

Merged
merged 2 commits into from
Jul 29, 2020

Conversation

Valarissa
Copy link
Contributor

This allows us to use non-inferrable regions with terraform and the vault provider.

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

Relates OR Closes #679

Release note for CHANGELOG:

Add support for passing region information stored in vault backend to AWS Config

Output from acceptance testing:
(Note: You need numerous environment variables to be set for these acceptance tests to run.
TF_ACC=1
VAULT_TOKEN=????

The following env vars must be set based on information provided by your AWS instance. In order to truly test these changes, you'll need to get Access Keys from the us-gov-west-1 cloud, as those credentials were the ones that didn't work which prompted this change (You can also use cn-northwest-1 to test this if you'd like).
AWS_ACCESS_KEY_ID=????
AWS_SECRET_ACCESS_KEY=????
AWS_DEFAULT_REGION=(cn-northwest-1|us-gov-west-1|us-east-1)

$ go test -run="TestAccDataSourceAWSAccessCredentials" -v ./vault

=== RUN   TestAccDataSourceAWSAccessCredentials_basic
--- PASS: TestAccDataSourceAWSAccessCredentials_basic (54.20s)
=== RUN   TestAccDataSourceAWSAccessCredentials_sts
--- PASS: TestAccDataSourceAWSAccessCredentials_sts (1.49s)
=== RUN   TestAccDataSourceAWSAccessCredentials_sts/sts_without_role_arn
    --- PASS: TestAccDataSourceAWSAccessCredentials_sts/sts_without_role_arn (0.77s)
=== RUN   TestAccDataSourceAWSAccessCredentials_sts/sts_with_role_arn
    --- PASS: TestAccDataSourceAWSAccessCredentials_sts/sts_with_role_arn (0.72s)
PASS

Process finished with exit code 0
...

I'd suggest running the tests with both us-east-1 and us-gov-west-1 credentials to ensure it all works.

This allows us to use non-inferrable regions
@ghost ghost added the size/S label Jul 27, 2020
Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

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

Minor nit, LGTM

vault/data_source_aws_access_credentials.go Outdated Show resolved Hide resolved
@Valarissa Valarissa merged commit f8b83fb into master Jul 29, 2020
@Valarissa Valarissa deleted the add_region_passing_for_non_commercial_regions branch July 29, 2020 17:36
@@ -155,9 +155,17 @@ func awsAccessCredentialsDataSourceRead(d *schema.ResourceData, meta interface{}
d.Set("lease_start_time", time.Now().Format(time.RFC3339))
d.Set("lease_renewable", secret.Renewable)

rootPath := backend + "/config/root"
regionData, err := client.Logical().Read(rootPath)

Choose a reason for hiding this comment

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

Uhm, this line appears to be a breaking change if existing source has not declared a region...

Copy link

Choose a reason for hiding this comment

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

I can confirm that this is a breaking change; I'm already getting reports of 405 errors from teams who haven't pinned their provider versions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for reporting this; we're tracking it in #833

@plaformsre
Copy link

Hello,

Can you please update the documentation and give an example?
https://www.terraform.io/docs/providers/vault/d/aws_access_credentials.html

Thank you,
Dejan

catsby added a commit that referenced this pull request Jul 31, 2020
PR 832 inadvertently introduced issues when the token policy did not
have the required permissions to read the root configuration.

This reverts commit f8b83fb.
catsby added a commit that referenced this pull request Jul 31, 2020
PR 832 inadvertently introduced issues when the token policy did not
have the required permissions to read the root configuration.

This reverts commit f8b83fb.
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
… AWS Config (hashicorp#832)

* Add support for passing region information to vault backend

This allows us to use non-inferrable regions

* Remove unnecessary data passing when obtaining region
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
PR 832 inadvertently introduced issues when the token policy did not
have the required permissions to read the root configuration.

This reverts commit f8b83fb.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic Secrets within China (cn-northwest-1)
5 participants