From a33d84ce2184d1ee1b58fbfbb2a36bfa4f467811 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 | 67 +++++------ aws/resource_aws_kms_key.go | 55 +++++---- aws/resource_aws_kms_key_test.go | 167 +++++++++++++++------------ website/docs/d/kms_key.html.markdown | 9 +- website/docs/r/kms_key.html.markdown | 6 +- 6 files changed, 171 insertions(+), 138 deletions(-) diff --git a/aws/data_source_aws_kms_key.go b/aws/data_source_aws_kms_key.go index 75dd23f119c5..4bd913f914f2 100644 --- a/aws/data_source_aws_kms_key.go +++ b/aws/data_source_aws_kms_key.go @@ -63,6 +63,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, @@ -103,6 +107,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 b736d8b8b1c4..193174c0cea1 100644 --- a/aws/data_source_aws_kms_key_test.go +++ b/aws/data_source_aws_kms_key_test.go @@ -2,31 +2,36 @@ package aws import ( "fmt" - "regexp" "testing" + "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) { + 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 8e67e0b271d2..13f1d6640007 100644 --- a/aws/resource_aws_kms_key.go +++ b/aws/resource_aws_kms_key.go @@ -44,11 +44,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": { @@ -82,13 +98,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)) } @@ -103,20 +119,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) + 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) @@ -133,7 +149,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) @@ -145,23 +161,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"), }) }) @@ -176,9 +191,9 @@ func resourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error { } d.Set("policy", policy) - out, err := retryOnAwsCode("NotFoundException", func() (interface{}, error) { + out, err := retryOnAwsCode(kms.ErrCodeNotFoundException, func() (interface{}, error) { return conn.GetKeyRotationStatus(&kms.GetKeyRotationStatusInput{ - KeyId: metadata.KeyId, + KeyId: aws.String(d.Id()), }) }) if err != nil { @@ -459,8 +474,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 c330502f4332..3272c7dda248 100644 --- a/aws/resource_aws_kms_key_test.go +++ b/aws/resource_aws_kms_key_test.go @@ -68,8 +68,8 @@ func testSweepKmsKeys(region string) error { } 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.ParallelTest(t, resource.TestCase{ @@ -80,7 +80,9 @@ func TestAccAWSKmsKey_basic(t *testing.T) { { Config: testAccAWSKmsKey(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists(resourceName, &keyBefore), + testAccCheckAWSKmsKeyExists(resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "customer_master_key_spec", "SYMMETRIC_DEFAULT"), + resource.TestCheckResourceAttr(resourceName, "key_usage", "ENCRYPT_DECRYPT"), ), }, { @@ -89,10 +91,26 @@ func TestAccAWSKmsKey_basic(t *testing.T) { 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_removedPolicy(rName), + Config: testAccAWSKmsKey_asymmetric(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists(resourceName, &keyAfter), + testAccCheckAWSKmsKeyExists(resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "customer_master_key_spec", "ECC_NIST_P384"), + resource.TestCheckResourceAttr(resourceName, "key_usage", "SIGN_VERIFY"), ), }, }, @@ -101,7 +119,7 @@ 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.ParallelTest(t, resource.TestCase{ @@ -113,11 +131,8 @@ func TestAccAWSKmsKey_disappears(t *testing.T) { Config: testAccAWSKmsKey(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsKeyExists(resourceName, &key), + testAccCheckAWSKmsKeyDisappears(&key), ), - }, - { - Config: testAccAWSKmsKey_other_region(rName), - PlanOnly: true, ExpectNonEmptyPlan: true, }, }, @@ -126,7 +141,7 @@ 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":"*"}]}` @@ -136,7 +151,7 @@ func TestAccAWSKmsKey_policy(t *testing.T) { CheckDestroy: testAccCheckAWSKmsKeyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsKey(rName), + Config: testAccAWSKmsKey_policy(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsKeyExists(resourceName, &key), testAccCheckAWSKmsKeyHasPolicy(resourceName, expectedPolicyText), @@ -148,13 +163,19 @@ func TestAccAWSKmsKey_policy(t *testing.T) { 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.ParallelTest(t, resource.TestCase{ @@ -165,8 +186,8 @@ func TestAccAWSKmsKey_isEnabled(t *testing.T) { { Config: testAccAWSKmsKey_enabledRotation(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists("aws_kms_key.test", &key1), - resource.TestCheckResourceAttr("aws_kms_key.test", "is_enabled", "true"), + testAccCheckAWSKmsKeyExists(resourceName, &key1), + resource.TestCheckResourceAttr(resourceName, "is_enabled", "true"), testAccCheckAWSKmsKeyIsEnabled(&key1, true), resource.TestCheckResourceAttr("aws_kms_key.test", "enable_key_rotation", "true"), ), @@ -180,19 +201,19 @@ func TestAccAWSKmsKey_isEnabled(t *testing.T) { { Config: testAccAWSKmsKey_disabled(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists("aws_kms_key.test", &key2), - resource.TestCheckResourceAttr("aws_kms_key.test", "is_enabled", "false"), + testAccCheckAWSKmsKeyExists(resourceName, &key2), + resource.TestCheckResourceAttr(resourceName, "is_enabled", "false"), testAccCheckAWSKmsKeyIsEnabled(&key2, false), - resource.TestCheckResourceAttr("aws_kms_key.test", "enable_key_rotation", "false"), + resource.TestCheckResourceAttr(resourceName, "enable_key_rotation", "false"), ), }, { Config: testAccAWSKmsKey_enabled(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists("aws_kms_key.test", &key3), - resource.TestCheckResourceAttr("aws_kms_key.test", "is_enabled", "true"), + testAccCheckAWSKmsKeyExists(resourceName, &key3), + resource.TestCheckResourceAttr(resourceName, "is_enabled", "true"), testAccCheckAWSKmsKeyIsEnabled(&key3, true), - resource.TestCheckResourceAttr("aws_kms_key.test", "enable_key_rotation", "true"), + resource.TestCheckResourceAttr(resourceName, "enable_key_rotation", "true"), ), }, }, @@ -200,8 +221,8 @@ 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.ParallelTest(t, resource.TestCase{ @@ -212,7 +233,7 @@ func TestAccAWSKmsKey_tags(t *testing.T) { { Config: testAccAWSKmsKey_tags(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists(resourceName, &keyBefore), + testAccCheckAWSKmsKeyExists(resourceName, &key), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), ), }, @@ -222,6 +243,13 @@ func TestAccAWSKmsKey_tags(t *testing.T) { ImportStateVerify: true, ImportStateVerifyIgnore: []string{"deletion_window_in_days"}, }, + { + Config: testAccAWSKmsKey(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSKmsKeyExists(resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + ), + }, }, }) } @@ -328,45 +356,44 @@ 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" "test" { - description = "Terraform acc test %s" + description = %[1]q deletion_window_in_days = 7 - - policy = <