From fac79d560a9d46cd203c905b35ae94d5b285010a Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 30 Jan 2018 12:01:23 -0500 Subject: [PATCH 1/2] data-source/aws_kms_alias: Prevent crash on AWS service aliases --- aws/data_source_aws_kms_alias.go | 30 ++++++----- aws/data_source_aws_kms_alias_test.go | 72 +++++++++++++++++++++------ 2 files changed, 75 insertions(+), 27 deletions(-) diff --git a/aws/data_source_aws_kms_alias.go b/aws/data_source_aws_kms_alias.go index e9d786f9536..9791af505ac 100644 --- a/aws/data_source_aws_kms_alias.go +++ b/aws/data_source_aws_kms_alias.go @@ -64,20 +64,24 @@ func dataSourceAwsKmsAliasRead(d *schema.ResourceData, meta interface{}) error { d.SetId(time.Now().UTC().String()) d.Set("arn", alias.AliasArn) - aliasARN, err := arn.Parse(*alias.AliasArn) - if err != nil { - return err - } - targetKeyARN := arn.ARN{ - Partition: aliasARN.Partition, - Service: aliasARN.Service, - Region: aliasARN.Region, - AccountID: aliasARN.AccountID, - Resource: fmt.Sprintf("key/%s", *alias.TargetKeyId), - } - d.Set("target_key_arn", targetKeyARN.String()) + // AWS service aliases do not return TargetKeyId: + // https://docs.aws.amazon.com/kms/latest/APIReference/API_ListAliases.html + if alias.TargetKeyId != nil { + aliasARN, err := arn.Parse(*alias.AliasArn) + if err != nil { + return err + } + targetKeyARN := arn.ARN{ + Partition: aliasARN.Partition, + Service: aliasARN.Service, + Region: aliasARN.Region, + AccountID: aliasARN.AccountID, + Resource: fmt.Sprintf("key/%s", *alias.TargetKeyId), + } + d.Set("target_key_arn", targetKeyARN.String()) - d.Set("target_key_id", alias.TargetKeyId) + d.Set("target_key_id", alias.TargetKeyId) + } return nil } diff --git a/aws/data_source_aws_kms_alias_test.go b/aws/data_source_aws_kms_alias_test.go index ee735e20995..f9bb17bf0bb 100644 --- a/aws/data_source_aws_kms_alias_test.go +++ b/aws/data_source_aws_kms_alias_test.go @@ -2,6 +2,7 @@ package aws import ( "fmt" + "regexp" "strings" "testing" @@ -10,32 +11,69 @@ import ( "github.com/hashicorp/terraform/terraform" ) -func TestAccDataSourceAwsKmsAlias(t *testing.T) { +func TestAccDataSourceAwsKmsAlias_AwsService(t *testing.T) { + name := "alias/aws/redshift" + resourceName := "data.aws_kms_alias.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccDataSourceAwsKmsAlias_name(name), + Check: resource.ComposeTestCheckFunc( + testAccDataSourceAwsKmsAliasCheckExists(resourceName), + resource.TestMatchResourceAttr(resourceName, "arn", regexp.MustCompile(fmt.Sprintf("^arn:[^:]+:kms:[^:]+:[^:]+:%s$", name))), + resource.TestCheckResourceAttr(resourceName, "name", name), + resource.TestCheckNoResourceAttr(resourceName, "target_key_arn"), + resource.TestCheckNoResourceAttr(resourceName, "target_key_id"), + ), + }, + }, + }) +} + +func TestAccDataSourceAwsKmsAlias_CMK(t *testing.T) { rInt := acctest.RandInt() + aliasResourceName := "aws_kms_alias.test" + datasourceAliasResourceName := "data.aws_kms_alias.test" + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, Steps: []resource.TestStep{ resource.TestStep{ - Config: testAccDataSourceAwsKmsAlias(rInt), + Config: testAccDataSourceAwsKmsAlias_CMK(rInt), Check: resource.ComposeTestCheckFunc( - testAccDataSourceAwsKmsAliasCheck("data.aws_kms_alias.by_name"), + testAccDataSourceAwsKmsAliasCheckExists(datasourceAliasResourceName), + testAccDataSourceAwsKmsAliasCheckCMKAttributes(aliasResourceName, datasourceAliasResourceName), ), }, }, }) } -func testAccDataSourceAwsKmsAliasCheck(name string) resource.TestCheckFunc { +func testAccDataSourceAwsKmsAliasCheckExists(name string) resource.TestCheckFunc { return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[name] + _, ok := s.RootModule().Resources[name] if !ok { return fmt.Errorf("root module has no resource called %s", name) } - kmsKeyRs, ok := s.RootModule().Resources["aws_kms_alias.single"] + return nil + } +} + +func testAccDataSourceAwsKmsAliasCheckCMKAttributes(aliasResourceName, datasourceAliasResourceName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[datasourceAliasResourceName] + if !ok { + return fmt.Errorf("root module has no resource called %s", datasourceAliasResourceName) + } + + kmsKeyRs, ok := s.RootModule().Resources[aliasResourceName] if !ok { - return fmt.Errorf("can't find aws_kms_alias.single in state") + return fmt.Errorf("can't find %s in state", aliasResourceName) } attr := rs.Primary.Attributes @@ -69,19 +107,25 @@ func testAccDataSourceAwsKmsAliasCheck(name string) resource.TestCheckFunc { } } -func testAccDataSourceAwsKmsAlias(rInt int) string { +func testAccDataSourceAwsKmsAlias_name(name string) string { + return fmt.Sprintf(` +data "aws_kms_alias" "test" { + name = "%s" +}`, name) +} + +func testAccDataSourceAwsKmsAlias_CMK(rInt int) string { return fmt.Sprintf(` -resource "aws_kms_key" "one" { +resource "aws_kms_key" "test" { description = "Terraform acc test" deletion_window_in_days = 7 } -resource "aws_kms_alias" "single" { +resource "aws_kms_alias" "test" { name = "alias/tf-acc-key-alias-%d" - target_key_id = "${aws_kms_key.one.key_id}" + target_key_id = "${aws_kms_key.test.key_id}" } -data "aws_kms_alias" "by_name" { - name = "${aws_kms_alias.single.name}" -}`, rInt) +%s +`, rInt, testAccDataSourceAwsKmsAlias_name("${aws_kms_alias.test.name}")) } From 19b34dff31a5161f7defd212bdd32680f7f5ff23 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Wed, 31 Jan 2018 08:57:42 -0500 Subject: [PATCH 2/2] data-source/aws_kms_alias: Improve comment about aliases missing TargetKeyId --- aws/data_source_aws_kms_alias.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/aws/data_source_aws_kms_alias.go b/aws/data_source_aws_kms_alias.go index 9791af505ac..c3a437904f9 100644 --- a/aws/data_source_aws_kms_alias.go +++ b/aws/data_source_aws_kms_alias.go @@ -64,7 +64,8 @@ func dataSourceAwsKmsAliasRead(d *schema.ResourceData, meta interface{}) error { d.SetId(time.Now().UTC().String()) d.Set("arn", alias.AliasArn) - // AWS service aliases do not return TargetKeyId: + // Some aliases do not return TargetKeyId (e.g. aliases for AWS services or + // aliases not associated with a Customer Managed Key (CMK)) // https://docs.aws.amazon.com/kms/latest/APIReference/API_ListAliases.html if alias.TargetKeyId != nil { aliasARN, err := arn.Parse(*alias.AliasArn)