From 638636e630fb4eb7471624bf64551a183e886647 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 29 Jan 2020 16:59:16 -0500 Subject: [PATCH] service/kms: Support asymmetric keys (#11062) Output from acceptance testing: ``` --- PASS: TestAccDataSourceAwsKmsKey_basic (28.69s) --- PASS: TestAccAWSKmsKey_disappears (7.85s) --- PASS: TestAccAWSKmsKey_asymmetricKey (28.29s) --- PASS: TestAccAWSKmsKey_basic (28.83s) --- PASS: TestAccAWSKmsKey_tags (33.60s) --- PASS: TestAccAWSKmsKey_policy (34.15s) --- PASS: TestAccAWSKmsKey_isEnabled (311.63s) ``` --- aws/data_source_aws_kms_key.go | 5 + aws/data_source_aws_kms_key_test.go | 73 +++----- aws/resource_aws_kms_key.go | 52 ++++-- aws/resource_aws_kms_key_test.go | 265 +++++++++++++++++---------- website/docs/d/kms_key.html.markdown | 9 +- website/docs/r/kms_key.html.markdown | 10 +- 6 files changed, 245 insertions(+), 169 deletions(-) diff --git a/aws/data_source_aws_kms_key.go b/aws/data_source_aws_kms_key.go index 35c231ead105..3e076e404824 100644 --- a/aws/data_source_aws_kms_key.go +++ b/aws/data_source_aws_kms_key.go @@ -62,6 +62,10 @@ func dataSourceAwsKmsKey() *schema.Resource { Type: schema.TypeString, Computed: true, }, + "customer_master_key_spec": { + Type: schema.TypeString, + Computed: true, + }, "origin": { Type: schema.TypeString, Computed: true, @@ -102,6 +106,7 @@ func dataSourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error { d.Set("key_manager", output.KeyMetadata.KeyManager) d.Set("key_state", output.KeyMetadata.KeyState) d.Set("key_usage", output.KeyMetadata.KeyUsage) + d.Set("customer_master_key_spec", output.KeyMetadata.CustomerMasterKeySpec) d.Set("origin", output.KeyMetadata.Origin) if output.KeyMetadata.ValidTo != nil { d.Set("valid_to", aws.TimeValue(output.KeyMetadata.ValidTo).Format(time.RFC3339)) diff --git a/aws/data_source_aws_kms_key_test.go b/aws/data_source_aws_kms_key_test.go index 6bb239ceb276..193174c0cea1 100644 --- a/aws/data_source_aws_kms_key_test.go +++ b/aws/data_source_aws_kms_key_test.go @@ -4,29 +4,34 @@ import ( "fmt" "testing" - "github.com/hashicorp/terraform/helper/resource" - "github.com/hashicorp/terraform/terraform" - "regexp" + "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/terraform" ) func TestAccDataSourceAwsKmsKey_basic(t *testing.T) { - resource.Test(t, resource.TestCase{ + resourceName := "aws_kms_key.test" + datasourceName := "data.aws_kms_key.test" + rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandStringFromCharSet(13, acctest.CharSetAlphaNum)) + + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, Steps: []resource.TestStep{ { - Config: testAccDataSourceAwsKmsKeyConfig, + Config: testAccDataSourceAwsKmsKeyConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccDataSourceAwsKmsKeyCheck("data.aws_kms_key.arbitrary"), - resource.TestMatchResourceAttr("data.aws_kms_key.arbitrary", "arn", regexp.MustCompile("^arn:[^:]+:kms:[^:]+:[^:]+:key/.+")), - resource.TestCheckResourceAttrSet("data.aws_kms_key.arbitrary", "aws_account_id"), - resource.TestCheckResourceAttrSet("data.aws_kms_key.arbitrary", "creation_date"), - resource.TestCheckResourceAttr("data.aws_kms_key.arbitrary", "description", "Terraform acc test"), - resource.TestCheckResourceAttr("data.aws_kms_key.arbitrary", "enabled", "true"), - resource.TestCheckResourceAttrSet("data.aws_kms_key.arbitrary", "key_manager"), - resource.TestCheckResourceAttrSet("data.aws_kms_key.arbitrary", "key_state"), - resource.TestCheckResourceAttr("data.aws_kms_key.arbitrary", "key_usage", "ENCRYPT_DECRYPT"), - resource.TestCheckResourceAttrSet("data.aws_kms_key.arbitrary", "origin"), + testAccDataSourceAwsKmsKeyCheck(datasourceName), + resource.TestCheckResourceAttrPair(datasourceName, "arn", resourceName, "arn"), + resource.TestCheckResourceAttrPair(datasourceName, "customer_master_key_spec", resourceName, "customer_master_key_spec"), + resource.TestCheckResourceAttrPair(datasourceName, "description", resourceName, "description"), + resource.TestCheckResourceAttrPair(datasourceName, "enabled", resourceName, "is_enabled"), + resource.TestCheckResourceAttrPair(datasourceName, "key_usage", resourceName, "key_usage"), + resource.TestCheckResourceAttrSet(datasourceName, "aws_account_id"), + resource.TestCheckResourceAttrSet(datasourceName, "creation_date"), + resource.TestCheckResourceAttrSet(datasourceName, "key_manager"), + resource.TestCheckResourceAttrSet(datasourceName, "key_state"), + resource.TestCheckResourceAttrSet(datasourceName, "origin"), ), }, }, @@ -35,41 +40,23 @@ func TestAccDataSourceAwsKmsKey_basic(t *testing.T) { func testAccDataSourceAwsKmsKeyCheck(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_key.arbitrary"] - if !ok { - return fmt.Errorf("can't find aws_kms_key.arbitrary in state") - } - - attr := rs.Primary.Attributes - - checkProperties := []string{"arn", "key_usage", "description"} - - for _, p := range checkProperties { - if attr[p] != kmsKeyRs.Primary.Attributes[p] { - return fmt.Errorf( - "%s is %s; want %s", - p, - attr[p], - kmsKeyRs.Primary.Attributes[p], - ) - } - } - return nil } } -const testAccDataSourceAwsKmsKeyConfig = ` -resource "aws_kms_key" "arbitrary" { - description = "Terraform acc test" - deletion_window_in_days = 7 +func testAccDataSourceAwsKmsKeyConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_kms_key" "test" { + description = %[1]q + deletion_window_in_days = 7 } -data "aws_kms_key" "arbitrary" { - key_id = "${aws_kms_key.arbitrary.key_id}" -}` +data "aws_kms_key" "test" { + key_id = "${aws_kms_key.test.key_id}" +}`, rName) +} diff --git a/aws/resource_aws_kms_key.go b/aws/resource_aws_kms_key.go index 78a1a03590c5..8d4ec2f4f5c4 100644 --- a/aws/resource_aws_kms_key.go +++ b/aws/resource_aws_kms_key.go @@ -49,11 +49,27 @@ func resourceAwsKmsKey() *schema.Resource { "key_usage": { Type: schema.TypeString, Optional: true, - Computed: true, + Default: kms.KeyUsageTypeEncryptDecrypt, ForceNew: true, ValidateFunc: validation.StringInSlice([]string{ - "", kms.KeyUsageTypeEncryptDecrypt, + kms.KeyUsageTypeSignVerify, + }, false), + }, + "customer_master_key_spec": { + Type: schema.TypeString, + Optional: true, + Default: kms.CustomerMasterKeySpecSymmetricDefault, + ForceNew: true, + ValidateFunc: validation.StringInSlice([]string{ + kms.CustomerMasterKeySpecSymmetricDefault, + kms.CustomerMasterKeySpecRsa2048, + kms.CustomerMasterKeySpecRsa3072, + kms.CustomerMasterKeySpecRsa4096, + kms.CustomerMasterKeySpecEccNistP256, + kms.CustomerMasterKeySpecEccNistP384, + kms.CustomerMasterKeySpecEccNistP521, + kms.CustomerMasterKeySpecEccSecgP256k1, }, false), }, "policy": { @@ -87,13 +103,13 @@ func resourceAwsKmsKeyCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).kmsconn // Allow aws to chose default values if we don't pass them - var req kms.CreateKeyInput + req := &kms.CreateKeyInput{ + CustomerMasterKeySpec: aws.String(d.Get("customer_master_key_spec").(string)), + KeyUsage: aws.String(d.Get("key_usage").(string)), + } if v, exists := d.GetOk("description"); exists { req.Description = aws.String(v.(string)) } - if v, exists := d.GetOk("key_usage"); exists { - req.KeyUsage = aws.String(v.(string)) - } if v, exists := d.GetOk("policy"); exists { req.Policy = aws.String(v.(string)) } @@ -108,17 +124,20 @@ func resourceAwsKmsKeyCreate(d *schema.ResourceData, meta interface{}) error { // http://docs.aws.amazon.com/kms/latest/APIReference/API_CreateKey.html err := resource.Retry(30*time.Second, func() *resource.RetryError { var err error - resp, err = conn.CreateKey(&req) - if isAWSErr(err, "MalformedPolicyDocumentException", "") { + resp, err = conn.CreateKey(req) + if isAWSErr(err, kms.ErrCodeMalformedPolicyDocumentException, "") { return resource.RetryableError(err) } return resource.NonRetryableError(err) }) + if isResourceTimeoutError(err) { + resp, err = conn.CreateKey(req) + } if err != nil { return err } - d.SetId(*resp.KeyMetadata.KeyId) + d.SetId(aws.StringValue(resp.KeyMetadata.KeyId)) d.Set("key_id", resp.KeyMetadata.KeyId) return resourceAwsKmsKeyUpdate(d, meta) @@ -135,7 +154,7 @@ func resourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error { var err error if d.IsNewResource() { var out interface{} - out, err = retryOnAwsCode("NotFoundException", func() (interface{}, error) { + out, err = retryOnAwsCode(kms.ErrCodeNotFoundException, func() (interface{}, error) { return conn.DescribeKey(req) }) resp, _ = out.(*kms.DescribeKeyOutput) @@ -147,23 +166,22 @@ func resourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error { } metadata := resp.KeyMetadata - if *metadata.KeyState == "PendingDeletion" { + if aws.StringValue(metadata.KeyState) == kms.KeyStatePendingDeletion { log.Printf("[WARN] Removing KMS key %s because it's already gone", d.Id()) d.SetId("") return nil } - d.SetId(*metadata.KeyId) - d.Set("arn", metadata.Arn) d.Set("key_id", metadata.KeyId) d.Set("description", metadata.Description) d.Set("key_usage", metadata.KeyUsage) + d.Set("customer_master_key_spec", metadata.CustomerMasterKeySpec) d.Set("is_enabled", metadata.Enabled) - pOut, err := retryOnAwsCode("NotFoundException", func() (interface{}, error) { + pOut, err := retryOnAwsCode(kms.ErrCodeNotFoundException, func() (interface{}, error) { return conn.GetKeyPolicy(&kms.GetKeyPolicyInput{ - KeyId: metadata.KeyId, + KeyId: aws.String(d.Id()), PolicyName: aws.String("default"), }) }) @@ -469,8 +487,8 @@ func resourceAwsKmsKeyDelete(d *schema.ResourceData, meta interface{}) error { // Wait for propagation since KMS is eventually consistent wait := resource.StateChangeConf{ - Pending: []string{"Enabled", "Disabled"}, - Target: []string{"PendingDeletion"}, + Pending: []string{kms.KeyStateEnabled, kms.KeyStateDisabled}, + Target: []string{kms.KeyStatePendingDeletion}, Timeout: 20 * time.Minute, MinTimeout: 2 * time.Second, ContinuousTargetOccurence: 10, diff --git a/aws/resource_aws_kms_key_test.go b/aws/resource_aws_kms_key_test.go index 9a98f893cd3c..61fba8f30073 100644 --- a/aws/resource_aws_kms_key_test.go +++ b/aws/resource_aws_kms_key_test.go @@ -90,8 +90,9 @@ func kmsTagHasPrefix(tags []*kms.Tag, key, prefix string) bool { } func TestAccAWSKmsKey_basic(t *testing.T) { - var keyBefore, keyAfter kms.KeyMetadata - rName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + var key kms.KeyMetadata + rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandStringFromCharSet(13, acctest.CharSetAlphaNum)) + resourceName := "aws_kms_key.test" resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -101,13 +102,37 @@ func TestAccAWSKmsKey_basic(t *testing.T) { { Config: testAccAWSKmsKey(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists("aws_kms_key.foo", &keyBefore), + testAccCheckAWSKmsKeyExists(resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "customer_master_key_spec", "SYMMETRIC_DEFAULT"), + resource.TestCheckResourceAttr(resourceName, "key_usage", "ENCRYPT_DECRYPT"), ), }, { - Config: testAccAWSKmsKey_removedPolicy(rName), + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"deletion_window_in_days"}, + }, + }, + }) +} + +func TestAccAWSKmsKey_asymmetricKey(t *testing.T) { + var key kms.KeyMetadata + rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandStringFromCharSet(13, acctest.CharSetAlphaNum)) + resourceName := "aws_kms_key.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSKmsKeyDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSKmsKey_asymmetric(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists("aws_kms_key.foo", &keyAfter), + testAccCheckAWSKmsKeyExists(resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "customer_master_key_spec", "ECC_NIST_P384"), + resource.TestCheckResourceAttr(resourceName, "key_usage", "SIGN_VERIFY"), ), }, }, @@ -116,7 +141,8 @@ func TestAccAWSKmsKey_basic(t *testing.T) { func TestAccAWSKmsKey_disappears(t *testing.T) { var key kms.KeyMetadata - rName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandStringFromCharSet(13, acctest.CharSetAlphaNum)) + resourceName := "aws_kms_key.test" resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -126,12 +152,9 @@ func TestAccAWSKmsKey_disappears(t *testing.T) { { Config: testAccAWSKmsKey(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists("aws_kms_key.foo", &key), + testAccCheckAWSKmsKeyExists(resourceName, &key), + testAccCheckAWSKmsKeyDisappears(&key), ), - }, - { - Config: testAccAWSKmsKey_other_region(rName), - PlanOnly: true, ExpectNonEmptyPlan: true, }, }, @@ -140,7 +163,8 @@ func TestAccAWSKmsKey_disappears(t *testing.T) { func TestAccAWSKmsKey_policy(t *testing.T) { var key kms.KeyMetadata - rName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandStringFromCharSet(13, acctest.CharSetAlphaNum)) + resourceName := "aws_kms_key.test" expectedPolicyText := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}` resource.Test(t, resource.TestCase{ @@ -149,19 +173,32 @@ func TestAccAWSKmsKey_policy(t *testing.T) { CheckDestroy: testAccCheckAWSKmsKeyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsKey(rName), + Config: testAccAWSKmsKey_policy(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsKeyExists("aws_kms_key.foo", &key), testAccCheckAWSKmsKeyHasPolicy("aws_kms_key.foo", expectedPolicyText), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"deletion_window_in_days"}, + }, + { + Config: testAccAWSKmsKey_removedPolicy(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSKmsKeyExists(resourceName, &key), + ), + }, }, }) } func TestAccAWSKmsKey_isEnabled(t *testing.T) { var key1, key2, key3 kms.KeyMetadata - rName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandStringFromCharSet(13, acctest.CharSetAlphaNum)) + resourceName := "aws_kms_key.test" resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -171,8 +208,8 @@ func TestAccAWSKmsKey_isEnabled(t *testing.T) { { Config: testAccAWSKmsKey_enabledRotation(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists("aws_kms_key.bar", &key1), - resource.TestCheckResourceAttr("aws_kms_key.bar", "is_enabled", "true"), + testAccCheckAWSKmsKeyExists(resourceName, &key1), + resource.TestCheckResourceAttr(resourceName, "is_enabled", "true"), testAccCheckAWSKmsKeyIsEnabled(&key1, true), resource.TestCheckResourceAttr("aws_kms_key.bar", "enable_key_rotation", "true"), ), @@ -180,19 +217,19 @@ func TestAccAWSKmsKey_isEnabled(t *testing.T) { { Config: testAccAWSKmsKey_disabled(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists("aws_kms_key.bar", &key2), - resource.TestCheckResourceAttr("aws_kms_key.bar", "is_enabled", "false"), + testAccCheckAWSKmsKeyExists(resourceName, &key2), + resource.TestCheckResourceAttr(resourceName, "is_enabled", "false"), testAccCheckAWSKmsKeyIsEnabled(&key2, false), - resource.TestCheckResourceAttr("aws_kms_key.bar", "enable_key_rotation", "false"), + resource.TestCheckResourceAttr(resourceName, "enable_key_rotation", "false"), ), }, { Config: testAccAWSKmsKey_enabled(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists("aws_kms_key.bar", &key3), - resource.TestCheckResourceAttr("aws_kms_key.bar", "is_enabled", "true"), + testAccCheckAWSKmsKeyExists(resourceName, &key3), + resource.TestCheckResourceAttr(resourceName, "is_enabled", "true"), testAccCheckAWSKmsKeyIsEnabled(&key3, true), - resource.TestCheckResourceAttr("aws_kms_key.bar", "enable_key_rotation", "true"), + resource.TestCheckResourceAttr(resourceName, "enable_key_rotation", "true"), ), }, }, @@ -200,8 +237,9 @@ func TestAccAWSKmsKey_isEnabled(t *testing.T) { } func TestAccAWSKmsKey_tags(t *testing.T) { - var keyBefore kms.KeyMetadata - rName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + var key kms.KeyMetadata + rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandStringFromCharSet(13, acctest.CharSetAlphaNum)) + resourceName := "aws_kms_key.test" resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -211,8 +249,21 @@ func TestAccAWSKmsKey_tags(t *testing.T) { { Config: testAccAWSKmsKey_tags(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists("aws_kms_key.foo", &keyBefore), - resource.TestCheckResourceAttr("aws_kms_key.foo", "tags.%", "3"), + testAccCheckAWSKmsKeyExists(resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"deletion_window_in_days"}, + }, + { + Config: testAccAWSKmsKey(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSKmsKeyExists(resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), ), }, }, @@ -321,43 +372,47 @@ func testAccCheckAWSKmsKeyIsEnabled(key *kms.KeyMetadata, isEnabled bool) resour } } +func testAccCheckAWSKmsKeyDisappears(key *kms.KeyMetadata) resource.TestCheckFunc { + return func(s *terraform.State) error { + conn := testAccProvider.Meta().(*AWSClient).kmsconn + + _, err := conn.ScheduleKeyDeletion(&kms.ScheduleKeyDeletionInput{ + KeyId: key.KeyId, + PendingWindowInDays: aws.Int64(int64(7)), + }) + + return err + } +} + func testAccAWSKmsKey(rName string) string { return fmt.Sprintf(` -resource "aws_kms_key" "foo" { - description = "Terraform acc test %s" - deletion_window_in_days = 7 - policy = <