data.aws_kms_alias: Use kms:DescribeKey to get target key Id & ARN

[handlerbot]

Context/fixes: #2009 (comment)

make testacc TESTARGS='-run=TestAccDataSourceAwsKmsAlias' passes.

cc @bflad @trung

[bflad]

@handlerbot wow this is awesome, good find! Is there a point to calling ListAliases at all? Seems like we should just skip ahead to DescribeKey where we allow passing in the name directly (and maybe allowing ARN as an optional argument for cross-account)

[bflad]

FYI I know there are a bunch of places in the repository currently using this, but we don't need the context wrapping of errwrap when we're just returning an error message. fmt.Errorf is just fine 😄

[handlerbot]

OK, got it. Want I should fix this, or merge and do a global sweep & cleanup later?

[bflad]

We'll do it later, no sense holding this up. 👍

[handlerbot]

@bflad I thought about that (giving up on ListAliases completely, per my comment herehttps://github.com//issues/2009#issuecomment-363072588), but I realized that you end up in kind of an annoying/pointless philosophical conundrum, and I actually ended up backing away from that conclusion slightly.

Here's how it goes:

1. There's no DescribeAlias, only DescribeKey or ListAliases.
2. If you call DescribeKey on an alias, you get all of the data about the underlying key the alias points to, but not the ARN of the alias.
3. The only way to get the ARN of the alias direct from AWS is ListAliases and walking through the list of names to match the one you asked for, and then getting the ARN from that entry.

So, we could switch this implementation to only calling DescribeKey, but then we'd have to synthesize the ARN of the alias, the same way the original code synthesizes the ARN of the key (when the target key id is available).

Thus: it really depends on how ideologically one feels about synthesizing ARNs (and risking there being bugs or needing a priority upgrade in the future if AWS changes something about ARNs suddenly) vs getting them direct from AWS responses without having to do any post-processing of them. I decided I liked the latter approach, at the expense of having to to do two API calls rather than one, so that's where I landed.

Also, I think you can't resolve KMS aliases cross-account, as there is no policy you can attach to aliases to specify that account X and Y have permission to resolve an alias to a key ID; when I have done this in Terraform, it was by creating an AWS provider for the second account, and using that provider for the data.aws_kms_alias resource...

[bflad]

LGTM! Thanks for finding this and contributing the fix!

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_AwsService
--- PASS: TestAccDataSourceAwsKmsAlias_AwsService (12.22s)
=== RUN   TestAccDataSourceAwsKmsAlias_CMK
--- PASS: TestAccDataSourceAwsKmsAlias_CMK (42.14s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	54.423s

[bflad]

This has been released in version 1.10.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[ghost]

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!