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

datasource/aws_kms_alias: Add target_key_arn attribute #2551

Merged
merged 1 commit into from
Jan 19, 2018

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Dec 5, 2017

This is an alternate implementation to #2224. Closes #2009.

KMS keys and aliases are bound to the same AWS partition, account, and region so it is safe to assemble the target key ARN from the alias ARN and target key ID. Reference: http://docs.aws.amazon.com/sdk-for-go/api/service/kms/#KMS.CreateAlias

The alias and the CMK it is mapped to must be in the same AWS account and the same region. You cannot perform this operation on an alias in a different AWS account.

make testacc TEST=./aws TESTARGS='-run=TestAccDataSourceAwsKmsAlias'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccDataSourceAwsKmsAlias -timeout 120m
=== RUN   TestAccDataSourceAwsKmsAlias
--- PASS: TestAccDataSourceAwsKmsAlias (41.76s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	41.791s

@handlerbot
Copy link
Contributor

@bflad Thank you so so much for this! I was meaning to add it myself after the second time I had to hand-assemble the target key ARN in my configs. :-D

@radeksimko radeksimko added the enhancement Requests to existing resources that expand the functionality or scope. label Dec 12, 2017
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

This looks overall good - just one nitpick about readability/cleanliness of the check.

Also do you mind resolving conflicts in the docs?

Thanks.

attr["target_key_arn"],
targetKeyArn,
)
}
Copy link
Member

Choose a reason for hiding this comment

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

How do you feel about regexp matching with $ here? It seems that would be a little bit cleaner approach compared to modification of the original attribute.

Copy link

@starlton-eb starlton-eb Jan 16, 2018

Choose a reason for hiding this comment

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

This would be awesome to have right now. I racked by brain all day wondering why my aws_efs_file_system instances were being recreated and discovered that it was due to a recent update to our stack that includes looking up a kms key:

resource "aws_efs_file_system" "efs" {
  creation_token = "${local.dbcluster}-efs-${var.env}"
  encrypted      = true
  kms_key_id     = "${data.aws_kms_alias.efs.arn}"

  tags {
    Name        = "${local.dbcluster}-efs-${var.env}"
    Environment = "${var.env}"
    Terraform   = "true"
  }
}

Learning more about this, what we really need is the target key's arn and not the alias

@radeksimko radeksimko added the waiting-response Maintainers are waiting on response from community or contributor. label Jan 8, 2018
@bflad bflad added this to the v1.8.0 milestone Jan 18, 2018
@bflad bflad added the service/kms Issues and PRs that pertain to the kms service. label Jan 18, 2018
@bflad
Copy link
Contributor Author

bflad commented Jan 18, 2018

I'll update and merge this PR after we release v1.7.1 (hopefully tomorrow). 🚀

@bflad bflad force-pushed the aws_kms_alias_target_key_arn branch from d6567a9 to 0008ef1 Compare January 18, 2018 20:10
@bflad
Copy link
Contributor Author

bflad commented Jan 18, 2018

Rebased and updated if you'd like to give another look

make testacc TEST=./aws TESTARGS='-run=TestAccDataSourceAwsKmsAlias'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccDataSourceAwsKmsAlias -timeout 120m
=== RUN   TestAccDataSourceAwsKmsAlias
--- PASS: TestAccDataSourceAwsKmsAlias (40.72s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	40.769s

@bflad bflad removed the waiting-response Maintainers are waiting on response from community or contributor. label Jan 18, 2018
@bflad bflad merged commit eb168d0 into hashicorp:master Jan 19, 2018
@bflad bflad deleted the aws_kms_alias_target_key_arn branch January 19, 2018 19:24
@bflad bflad changed the title d/aws_kms_alias: Add target_key_arn attribute datasource/aws_kms_alias: Add target_key_arn attribute Jan 19, 2018
bflad added a commit that referenced this pull request Jan 19, 2018
@ColinHebert
Copy link
Contributor

Helps/fixes #3019

@bflad
Copy link
Contributor Author

bflad commented Jan 29, 2018

This has been released in terraform-provider-aws version 1.8.0. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

@ghost
Copy link

ghost commented Apr 8, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/kms Issues and PRs that pertain to the kms service.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support ARNs for existing KMS keys
5 participants