From 5d3f9dc1505ae5b763ef3b9e4ad9e5af4844a983 Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Fri, 25 Jun 2021 15:56:33 +0300 Subject: [PATCH 1/5] add `multi_region` --- aws/resource_aws_kms_key.go | 68 +++++++++++++++----------------- aws/resource_aws_kms_key_test.go | 55 +++++++++++++++++++------- 2 files changed, 72 insertions(+), 51 deletions(-) diff --git a/aws/resource_aws_kms_key.go b/aws/resource_aws_kms_key.go index 590a88b4e3b..37b05687b69 100644 --- a/aws/resource_aws_kms_key.go +++ b/aws/resource_aws_kms_key.go @@ -6,7 +6,6 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/kms" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -40,35 +39,24 @@ func resourceAwsKmsKey() *schema.Resource { Computed: true, }, "description": { - Type: schema.TypeString, - Optional: true, - Computed: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateFunc: validation.StringLenBetween(0, 8192), }, "key_usage": { - Type: schema.TypeString, - Optional: true, - Default: kms.KeyUsageTypeEncryptDecrypt, - ForceNew: true, - ValidateFunc: validation.StringInSlice([]string{ - kms.KeyUsageTypeEncryptDecrypt, - kms.KeyUsageTypeSignVerify, - }, false), + Type: schema.TypeString, + Optional: true, + Default: kms.KeyUsageTypeEncryptDecrypt, + ForceNew: true, + ValidateFunc: validation.StringInSlice(kms.KeyUsageType_Values(), 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), + Type: schema.TypeString, + Optional: true, + Default: kms.CustomerMasterKeySpecSymmetricDefault, + ForceNew: true, + ValidateFunc: validation.StringInSlice(kms.CustomerMasterKeySpec_Values(), false), }, "policy": { Type: schema.TypeString, @@ -82,6 +70,12 @@ func resourceAwsKmsKey() *schema.Resource { Optional: true, Default: true, }, + "multi_region": { + Type: schema.TypeBool, + Optional: true, + ForceNew: true, + Default: false, + }, "enable_key_rotation": { Type: schema.TypeBool, Optional: true, @@ -107,6 +101,7 @@ func resourceAwsKmsKeyCreate(d *schema.ResourceData, meta interface{}) error { req := &kms.CreateKeyInput{ CustomerMasterKeySpec: aws.String(d.Get("customer_master_key_spec").(string)), KeyUsage: aws.String(d.Get("key_usage").(string)), + MultiRegion: aws.Bool(d.Get("multi_region").(bool)), } if v, exists := d.GetOk("description"); exists { req.Description = aws.String(v.(string)) @@ -196,6 +191,7 @@ func resourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error { d.Set("key_usage", metadata.KeyUsage) d.Set("customer_master_key_spec", metadata.CustomerMasterKeySpec) d.Set("is_enabled", metadata.Enabled) + d.Set("multi_region", metadata.MultiRegion) pOut, err := retryOnAwsCode(kms.ErrCodeNotFoundException, func() (interface{}, error) { return conn.GetKeyPolicy(&kms.GetKeyPolicyInput{ @@ -210,7 +206,7 @@ func resourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error { p := pOut.(*kms.GetKeyPolicyOutput) policy, err := structure.NormalizeJsonString(*p.Policy) if err != nil { - return fmt.Errorf("policy contains an invalid JSON: %s", err) + return fmt.Errorf("policy contains an invalid JSON: %w", err) } d.Set("policy", policy) @@ -246,7 +242,7 @@ func resourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error { } if err != nil { - return fmt.Errorf("error listing tags for KMS Key (%s): %s", d.Id(), err) + return fmt.Errorf("error listing tags for KMS Key (%s): %w", d.Id(), err) } tags = tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig) @@ -302,7 +298,7 @@ func resourceAwsKmsKeyUpdate(d *schema.ResourceData, meta interface{}) error { o, n := d.GetChange("tags_all") if err := keyvaluetags.KmsUpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("error updating KMS Key (%s) tags: %s", d.Id(), err) + return fmt.Errorf("error updating KMS Key (%s) tags: %w", d.Id(), err) } } @@ -327,7 +323,7 @@ func resourceAwsKmsKeyDescriptionUpdate(conn *kms.KMS, d *schema.ResourceData) e func resourceAwsKmsKeyPolicyUpdate(conn *kms.KMS, d *schema.ResourceData) error { policy, err := structure.NormalizeJsonString(d.Get("policy").(string)) if err != nil { - return fmt.Errorf("policy contains an invalid JSON: %s", err) + return fmt.Errorf("policy contains an invalid JSON: %w", err) } keyId := d.Get("key_id").(string) @@ -377,8 +373,7 @@ func updateKmsKeyStatus(conn *kms.KMS, id string, shouldBeEnabled bool) error { KeyId: aws.String(id), }) if err != nil { - awsErr, ok := err.(awserr.Error) - if ok && awsErr.Code() == "NotFoundException" { + if isAWSErr(err, kms.ErrCodeNotFoundException, "") { return nil, fmt.Sprintf("%t", !shouldBeEnabled), nil } return resp, "FAILED", err @@ -392,7 +387,7 @@ func updateKmsKeyStatus(conn *kms.KMS, id string, shouldBeEnabled bool) error { _, err = wait.WaitForState() if err != nil { - return fmt.Errorf("Failed setting KMS key status to %t: %s", shouldBeEnabled, err) + return fmt.Errorf("Failed setting KMS key status to %t: %w", shouldBeEnabled, err) } return nil @@ -405,11 +400,10 @@ func updateKmsKeyRotationStatus(conn *kms.KMS, d *schema.ResourceData) error { err := handleKeyRotation(conn, shouldEnableRotation, aws.String(d.Id())) if err != nil { - awsErr, ok := err.(awserr.Error) - if ok && awsErr.Code() == "DisabledException" { + if isAWSErr(err, kms.ErrCodeDisabledException, "") { return resource.RetryableError(err) } - if ok && awsErr.Code() == "NotFoundException" { + if isAWSErr(err, kms.ErrCodeNotFoundException, "") { return resource.RetryableError(err) } @@ -438,7 +432,7 @@ func updateKmsKeyRotationStatus(conn *kms.KMS, d *schema.ResourceData) error { log.Printf("[DEBUG] Checking if KMS key %s rotation status is %t", d.Id(), shouldEnableRotation) - out, err := retryOnAwsCode("NotFoundException", func() (interface{}, error) { + out, err := retryOnAwsCode(kms.ErrCodeNotFoundException, func() (interface{}, error) { return conn.GetKeyRotationStatus(&kms.GetKeyRotationStatusInput{ KeyId: aws.String(d.Id()), }) diff --git a/aws/resource_aws_kms_key_test.go b/aws/resource_aws_kms_key_test.go index 300f0ffffb2..cd5912940d2 100644 --- a/aws/resource_aws_kms_key_test.go +++ b/aws/resource_aws_kms_key_test.go @@ -84,6 +84,8 @@ func TestAccAWSKmsKey_basic(t *testing.T) { testAccCheckAWSKmsKeyExists(resourceName, &key), resource.TestCheckResourceAttr(resourceName, "customer_master_key_spec", "SYMMETRIC_DEFAULT"), resource.TestCheckResourceAttr(resourceName, "key_usage", "ENCRYPT_DECRYPT"), + resource.TestCheckResourceAttr(resourceName, "multi_region", "false"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), ), }, { @@ -134,7 +136,7 @@ func TestAccAWSKmsKey_disappears(t *testing.T) { Config: testAccAWSKmsKey(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsKeyExists(resourceName, &key), - testAccCheckAWSKmsKeyDisappears(&key), + testAccCheckResourceDisappears(testAccProvider, resourceAwsKmsKey(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -315,6 +317,34 @@ func TestAccAWSKmsKey_tags(t *testing.T) { }) } +func TestAccAWSKmsKey_multiRegion(t *testing.T) { + var key kms.KeyMetadata + rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandString(13)) + resourceName := "aws_kms_key.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, kms.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSKmsKeyDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSKmsKeyMultiRegionConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSKmsKeyExists(resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "multi_region", "true"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"deletion_window_in_days"}, + }, + }, + }) +} + func testAccCheckAWSKmsKeyHasPolicy(name string, expectedPolicyText string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[name] @@ -417,19 +447,6 @@ 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" { @@ -656,3 +673,13 @@ resource "aws_kms_key" "test" { } `, rName) } + +func testAccAWSKmsKeyMultiRegionConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_kms_key" "test" { + description = %[1]q + deletion_window_in_days = 7 + multi_region = true +} +`, rName) +} From 584f3aacb12b1acbbd56041f2dd1544ab71c609a Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Fri, 25 Jun 2021 15:58:07 +0300 Subject: [PATCH 2/5] docs --- website/docs/r/kms_key.html.markdown | 1 + 1 file changed, 1 insertion(+) diff --git a/website/docs/r/kms_key.html.markdown b/website/docs/r/kms_key.html.markdown index 7248feffd5b..5dbcbdee834 100644 --- a/website/docs/r/kms_key.html.markdown +++ b/website/docs/r/kms_key.html.markdown @@ -35,6 +35,7 @@ Valid values: `SYMMETRIC_DEFAULT`, `RSA_2048`, `RSA_3072`, `RSA_4096`, `ECC_NIS * `deletion_window_in_days` - (Optional) Duration in days after which the key is deleted after destruction of the resource, must be between 7 and 30 days. Defaults to 30 days. * `is_enabled` - (Optional) Specifies whether the key is enabled. Defaults to true. * `enable_key_rotation` - (Optional) Specifies whether [key rotation](http://docs.aws.amazon.com/kms/latest/developerguide/rotate-keys.html) is enabled. Defaults to false. +* `multi_region` - (Optional) Creates a multi-Region primary key that you can replicate into other AWS Regions. You cannot change this value after you create the CMK. Defaults to false. * `tags` - (Optional) A map of tags to assign to the object. If configured with a provider [`default_tags` configuration block](https://www.terraform.io/docs/providers/aws/index.html#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. ## Attributes Reference From 6af2e26ebba4ab7b8923f63a176a2cd46b370740 Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Fri, 25 Jun 2021 16:01:29 +0300 Subject: [PATCH 3/5] change log --- .changelog/19967.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changelog/19967.txt diff --git a/.changelog/19967.txt b/.changelog/19967.txt new file mode 100644 index 00000000000..1c64698511d --- /dev/null +++ b/.changelog/19967.txt @@ -0,0 +1,7 @@ +```release-note:enhancement +resource/aws_kms_key: Add `multi_region` argument. +``` + +```release-note:enhancement +resource/aws_kms_key: Add plan time validation to `description`. +``` From 490b7400ef89f878e65babc86eaad063da5c73dc Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Fri, 25 Jun 2021 16:12:38 +0300 Subject: [PATCH 4/5] sweeper --- aws/resource_aws_kms_key_test.go | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/aws/resource_aws_kms_key_test.go b/aws/resource_aws_kms_key_test.go index cd5912940d2..3d33f1acadb 100644 --- a/aws/resource_aws_kms_key_test.go +++ b/aws/resource_aws_kms_key_test.go @@ -23,34 +23,37 @@ func init() { func testSweepKmsKeys(region string) error { client, err := sharedClientForRegion(region) if err != nil { - return fmt.Errorf("error getting client: %s", err) + return fmt.Errorf("error getting client: %w", err) } conn := client.(*AWSClient).kmsconn err = conn.ListKeysPages(&kms.ListKeysInput{Limit: aws.Int64(int64(1000))}, func(out *kms.ListKeysOutput, lastPage bool) bool { for _, k := range out.Keys { + kKeyId := aws.StringValue(k.KeyId) kOut, err := conn.DescribeKey(&kms.DescribeKeyInput{ KeyId: k.KeyId, }) if err != nil { - log.Printf("Error: Failed to describe key %q: %s", *k.KeyId, err) + log.Printf("Error: Failed to describe key %q: %s", kKeyId, err) return false } - if *kOut.KeyMetadata.KeyManager == kms.KeyManagerTypeAws { + if aws.StringValue(kOut.KeyMetadata.KeyManager) == kms.KeyManagerTypeAws { // Skip (default) keys which are managed by AWS continue } - if *kOut.KeyMetadata.KeyState == kms.KeyStatePendingDeletion { + if aws.StringValue(kOut.KeyMetadata.KeyState) == kms.KeyStatePendingDeletion { // Skip keys which are already scheduled for deletion continue } - _, err = conn.ScheduleKeyDeletion(&kms.ScheduleKeyDeletionInput{ - KeyId: k.KeyId, - PendingWindowInDays: aws.Int64(int64(7)), - }) + r := resourceAwsKmsKey() + d := r.Data(nil) + d.SetId(kKeyId) + d.Set("key_id", kKeyId) + d.Set("deletion_window_in_days", "7") + err = r.Delete(d, client) if err != nil { - log.Printf("Error: Failed to schedule key %q for deletion: %s", *k.KeyId, err) + log.Printf("Error: Failed to schedule key %q for deletion: %s", kKeyId, err) return false } } @@ -61,7 +64,7 @@ func testSweepKmsKeys(region string) error { log.Printf("[WARN] Skipping KMS Key sweep for %s: %s", region, err) return nil } - return fmt.Errorf("Error describing KMS keys: %s", err) + return fmt.Errorf("Error describing KMS keys: %w", err) } return nil From 19f18ba8a65552b2fcc1ecc537d41cb41f69addc Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 15 Jul 2021 11:26:17 -0400 Subject: [PATCH 5/5] r/aws_kms_key: Remove 'multi_region' attribute. Multi-Region CMKs may require their own resource type. --- .changelog/19967.txt | 4 --- aws/resource_aws_kms_key.go | 8 ------ aws/resource_aws_kms_key_test.go | 39 ---------------------------- website/docs/r/kms_key.html.markdown | 3 +-- 4 files changed, 1 insertion(+), 53 deletions(-) diff --git a/.changelog/19967.txt b/.changelog/19967.txt index 1c64698511d..98b176d6c44 100644 --- a/.changelog/19967.txt +++ b/.changelog/19967.txt @@ -1,7 +1,3 @@ -```release-note:enhancement -resource/aws_kms_key: Add `multi_region` argument. -``` - ```release-note:enhancement resource/aws_kms_key: Add plan time validation to `description`. ``` diff --git a/aws/resource_aws_kms_key.go b/aws/resource_aws_kms_key.go index 37b05687b69..586a7839fb9 100644 --- a/aws/resource_aws_kms_key.go +++ b/aws/resource_aws_kms_key.go @@ -70,12 +70,6 @@ func resourceAwsKmsKey() *schema.Resource { Optional: true, Default: true, }, - "multi_region": { - Type: schema.TypeBool, - Optional: true, - ForceNew: true, - Default: false, - }, "enable_key_rotation": { Type: schema.TypeBool, Optional: true, @@ -101,7 +95,6 @@ func resourceAwsKmsKeyCreate(d *schema.ResourceData, meta interface{}) error { req := &kms.CreateKeyInput{ CustomerMasterKeySpec: aws.String(d.Get("customer_master_key_spec").(string)), KeyUsage: aws.String(d.Get("key_usage").(string)), - MultiRegion: aws.Bool(d.Get("multi_region").(bool)), } if v, exists := d.GetOk("description"); exists { req.Description = aws.String(v.(string)) @@ -191,7 +184,6 @@ func resourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error { d.Set("key_usage", metadata.KeyUsage) d.Set("customer_master_key_spec", metadata.CustomerMasterKeySpec) d.Set("is_enabled", metadata.Enabled) - d.Set("multi_region", metadata.MultiRegion) pOut, err := retryOnAwsCode(kms.ErrCodeNotFoundException, func() (interface{}, error) { return conn.GetKeyPolicy(&kms.GetKeyPolicyInput{ diff --git a/aws/resource_aws_kms_key_test.go b/aws/resource_aws_kms_key_test.go index 3d33f1acadb..fce4b5b8a3f 100644 --- a/aws/resource_aws_kms_key_test.go +++ b/aws/resource_aws_kms_key_test.go @@ -87,7 +87,6 @@ func TestAccAWSKmsKey_basic(t *testing.T) { testAccCheckAWSKmsKeyExists(resourceName, &key), resource.TestCheckResourceAttr(resourceName, "customer_master_key_spec", "SYMMETRIC_DEFAULT"), resource.TestCheckResourceAttr(resourceName, "key_usage", "ENCRYPT_DECRYPT"), - resource.TestCheckResourceAttr(resourceName, "multi_region", "false"), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), ), }, @@ -320,34 +319,6 @@ func TestAccAWSKmsKey_tags(t *testing.T) { }) } -func TestAccAWSKmsKey_multiRegion(t *testing.T) { - var key kms.KeyMetadata - rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandString(13)) - resourceName := "aws_kms_key.test" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ErrorCheck: testAccErrorCheck(t, kms.EndpointsID), - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSKmsKeyDestroy, - Steps: []resource.TestStep{ - { - Config: testAccAWSKmsKeyMultiRegionConfig(rName), - Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsKeyExists(resourceName, &key), - resource.TestCheckResourceAttr(resourceName, "multi_region", "true"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"deletion_window_in_days"}, - }, - }, - }) -} - func testAccCheckAWSKmsKeyHasPolicy(name string, expectedPolicyText string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[name] @@ -676,13 +647,3 @@ resource "aws_kms_key" "test" { } `, rName) } - -func testAccAWSKmsKeyMultiRegionConfig(rName string) string { - return fmt.Sprintf(` -resource "aws_kms_key" "test" { - description = %[1]q - deletion_window_in_days = 7 - multi_region = true -} -`, rName) -} diff --git a/website/docs/r/kms_key.html.markdown b/website/docs/r/kms_key.html.markdown index 5dbcbdee834..0a063d31a6b 100644 --- a/website/docs/r/kms_key.html.markdown +++ b/website/docs/r/kms_key.html.markdown @@ -8,7 +8,7 @@ description: |- # Resource: aws_kms_key -Provides a KMS customer master key. +Provides a KMS single-Region customer master key (CMK). ## Example Usage @@ -35,7 +35,6 @@ Valid values: `SYMMETRIC_DEFAULT`, `RSA_2048`, `RSA_3072`, `RSA_4096`, `ECC_NIS * `deletion_window_in_days` - (Optional) Duration in days after which the key is deleted after destruction of the resource, must be between 7 and 30 days. Defaults to 30 days. * `is_enabled` - (Optional) Specifies whether the key is enabled. Defaults to true. * `enable_key_rotation` - (Optional) Specifies whether [key rotation](http://docs.aws.amazon.com/kms/latest/developerguide/rotate-keys.html) is enabled. Defaults to false. -* `multi_region` - (Optional) Creates a multi-Region primary key that you can replicate into other AWS Regions. You cannot change this value after you create the CMK. Defaults to false. * `tags` - (Optional) A map of tags to assign to the object. If configured with a provider [`default_tags` configuration block](https://www.terraform.io/docs/providers/aws/index.html#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. ## Attributes Reference