data-source/aws_kms_alias: Prevent crash on aliases without target key #3203
Conversation
bflad
added
bug
There was a problem hiding this comment.
Hey @bflad,
Good job fixing that! 🚀 🚗
Just left a nitpick, non-blocking at all to me!
$ 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 (24.56s)
=== RUN TestAccDataSourceAwsKmsAlias_CMK
--- PASS: TestAccDataSourceAwsKmsAlias_CMK (54.35s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 78.963s
aws/data_source_aws_kms_alias.go
Outdated
| Resource: fmt.Sprintf("key/%s", *alias.TargetKeyId), | ||
| } | ||
| d.Set("target_key_arn", targetKeyARN.String()) | ||
| // AWS service aliases do not return TargetKeyId: |
There was a problem hiding this comment.
Nitpick: I would probably enhance this message so that we add the context ... do not return TargetKeyId when not associated with a Customer Managed Key (CMK). What do you think?
There was a problem hiding this comment.
Agree with @Ninir, there are cases where the key is associated, it is just not the case all the time
There was a problem hiding this comment.
I'll update this comment 👍
|
Sweet! I know it's a bit demanding but can we get that one released in a 1.8.1 release? |
bflad
changed the title
|
This has been released in terraform-provider-aws version 1.9.0. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
|
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
locked and limited conversation to collaborators
According to the KMS API documentation, AWS service KMS aliases (e.g.
alias/aws/redshift) do not return TargetKeyId. This PR adds testing against those aliases and ensures the data source does not returntarget_key_arnortarget_key_idin that scenario.New acceptance test without data source changes:
After: