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

Support ARNs for existing KMS keys #2009

Closed
duncanhall opened this issue Oct 23, 2017 · 9 comments · Fixed by #2224, #3304 or #2551
Closed

Support ARNs for existing KMS keys #2009

duncanhall opened this issue Oct 23, 2017 · 9 comments · Fixed by #2224, #3304 or #2551
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/kms Issues and PRs that pertain to the kms service.
Milestone

Comments

@duncanhall
Copy link
Contributor

There is currently no way to directly retrieve an ARN for a pre-existing KMS Key. This makes it difficult / unnecessarily complicated to assign existing KMS keys to newly created resources.

Terraform Version

0.10.7

Affected Resource(s)

  • aws_kms_alias
  • aws_lambda_function
  • Any resource which accepts a kms_key_arn argument

Terraform Configuration Files

data "aws_kms_alias" "existing_cmk" {                                                                                                                                                                         
  name = "alias/MyPreExistingKey"                                                                                                                                                                        
}

resource "aws_lambda_function" "new_function" {
  /* other arguments ommitted */
  kms_key_arn = "${data.aws_kms_alias.existing_cmk.arn}"
}  

Expected Behavior

The newly created Lambda function would have the MyPreExisting KMS key set.

Actual Behavior

The Lambda function gets assigned either no KMS key, or the default aws/lambda key.

Important Factoids

After failing to have the expected KMS key assigned, I looked at the plan output and saw that the value of ${data.aws_kms_alias.existing_cmk.arn} was in the format of

arn:aws:kms:eu-west-1:123456789012:alias/MyExistingKey

This is the ARN of the alias, not the key itself, which are seemingly not analogous. Logically this makes sense, but as there is no aws_kms_key data provider, there is no way to directly retrieve an ARN for an existing key.

You can indirectly solve this by using some substitution, but it would be nice not to have to do this:

locals {                                                                                                                                                                                                    
  kms_arn = "${replace(data.aws_kms_alias.existing_cmk.arn, data.aws_kms_alias.existing_cmk.name, "key/${data.aws_kms_alias.existing_cmk.target_key_id}")}"                                                       
}  
@trung
Copy link
Contributor

trung commented Oct 23, 2017

aws_kms_alias uses ListAliases API (http://docs.aws.amazon.com/kms/latest/APIReference/API_ListAliases.html) which doesn't output Key ARN and also there's a potential miss if pagination is encountered.

Need a new data source that calls DescribeKey API (http://docs.aws.amazon.com/kms/latest/APIReference/API_DescribeKey.html), it would fit the use case you describe above.
I can work on the PR

Updated: DescribeKey is only for CMK

@radeksimko radeksimko added the enhancement Requests to existing resources that expand the functionality or scope. label Oct 23, 2017
trung added a commit to trung/terraform-provider-aws that referenced this issue Nov 8, 2017
trung added a commit to trung/terraform-provider-aws that referenced this issue Nov 8, 2017
trung added a commit to trung/terraform-provider-aws that referenced this issue Nov 9, 2017
@bflad
Copy link
Contributor

bflad commented Dec 5, 2017

In addition to #2224 for a aws_kms_key data source, I've also submitted #2551 to add a target_key_arn attribute to the aws_kms_alias data source (analogous to locals workaround in description).

@bflad bflad added this to the v1.8.0 milestone Jan 18, 2018
@bflad
Copy link
Contributor

bflad commented Jan 19, 2018

The target_key_arn enhancement to aws_kms_alias data source has been merged into master and will release with v1.8.0.

@bflad bflad added the service/kms Issues and PRs that pertain to the kms service. label Jan 19, 2018
@bflad
Copy link
Contributor

bflad commented Jan 25, 2018

target_key_arn attribute is now also available on the aws_kms_alias resource in master and will ship with v1.8.0 (ETA end of week).

@bflad
Copy link
Contributor

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.

@handlerbot
Copy link
Contributor

handlerbot commented Feb 5, 2018

@bflad, in light of #3189 and #3203 I wanted to discuss this feature, and suggest consideration of #2224.

I think the issue described in #3189 is actually an AWS API bug which needs to be reported to AWS for fixing. (Which I am glad to do.)

From experimental observation, I think that whether the KMS API call ListAliases returns the target key id for the alias in question is actually a question of an internal cache to AWS (!!). If you query for the details of the alias via KMS API call DescribeKey (which actually works just fine on aliases, despite the name), it appears to fill the cache, and then ListAliases starts returning the target key id:

>>> kms = session.client("kms")

>>> [x["AliasName"] for x in kms.list_aliases()["Aliases"] if "TargetKeyId" not in x]
['alias/aws/dynamodb', 'alias/aws/ebs', 'alias/aws/kinesisvideo', 'alias/aws/redshift', 'alias/aws/ssm']

>>> len(kms.describe_key(KeyId="alias/aws/ebs")["KeyMetadata"]["KeyId"])
36

>>> [x["AliasName"] for x in kms.list_aliases()["Aliases"] if "TargetKeyId" not in x]
['alias/aws/dynamodb', 'alias/aws/kinesisvideo', 'alias/aws/redshift', 'alias/aws/ssm']

>>> len(kms.describe_key(KeyId="alias/aws/ssm")["KeyMetadata"]["KeyId"])
36

>>> [x["AliasName"] for x in kms.list_aliases()["Aliases"] if "TargetKeyId" not in x]
['alias/aws/dynamodb', 'alias/aws/kinesisvideo', 'alias/aws/redshift']

>>> len(kms.describe_key(KeyId="alias/aws/dynamodb")["KeyMetadata"]["KeyId"])
36

>>> [x["AliasName"] for x in kms.list_aliases()["Aliases"] if "TargetKeyId" not in x]
['alias/aws/kinesisvideo', 'alias/aws/redshift']

Given:

  1. ListAliases is apparently eventually/partially consistent, or at least buggy at this time.
  2. DescribeKey is fully consistent, and works right now.
  3. Looking up one entity directly that you already know the name of is preferable to requesting all aliases and iterating over the list until you (maybe) find the one you're looking for, as is the current implementation of data source aws_kms_alias.

Thus, I think:

  1. AWS data source aws_kms_alias should be reworked to use DescribeKey instead of ListAliases.

  2. New Data Source: aws_kms_key #2224 should be considered as important, as it gives a reliable source of all data fields for both KMS keys and aliases. (Maybe the resource in that PR should be renamed to aws_kms_entity or aws_kms_key_or_alias or the like, to make it clear from the name that it can be used for both keys and aliases.)

I'm glad to work up a PR for 1); would it be welcomed / accepted? Thoughts in general?

(cc @trung re: his PR)

Update 1: I have reported the ListAliases bug to AWS Support.

Update 2: Response from AWS Support:

The reason why you don't see a TargetKeyID is that they are service keys that haven't yet been auto-generated for you. If you take a look at the API description here
(https://docs.aws.amazon.com/kms/latest/APIReference/API_ListAliases.html) we mention that the response might include several aliases that do not have a TargetKeyID because they are not associated with a CMK. If you take a look at the alias they are probably services that you haven't used yet in that region. After you call DescribeKey you will have a CMK associated, then yes you will see a TargetKeyID as you mentioned.

@bflad
Copy link
Contributor

bflad commented Feb 14, 2018

Nice investigative work here! I think we should switch the data source to DescribeKey only if possible in #3304

@bflad
Copy link
Contributor

bflad commented Feb 14, 2018

As a followup to when #2224 lands (reviewing it now), we might be able to alias (pardon the pun) the aws_kms_alias data source with the aws_kms_key data source. We would just need to handle mapping the target_key_id and target_key_arn attributes over to not introduce a backwards incompatible change.

trung added a commit to trung/terraform-provider-aws that referenced this issue Feb 26, 2018
trung added a commit to trung/terraform-provider-aws that referenced this issue Feb 26, 2018
@ghost
Copy link

ghost commented Apr 7, 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 7, 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
5 participants