Skip to content

Commit

Permalink
resource/aws_kms_key: Prevent eventual consistency related errors on …
Browse files Browse the repository at this point in the history
…creation

Reference: hashicorp#9953
Reference: hashicorp#11781
Reference: hashicorp#12427 (comment)

This refactors the resource logic to prevent `Update` after `Create` type logic errors with duplicate API calls (potential error points for eventual consistency):

- Setting `description` on creation previously was done once during the `CreateKey` call and again via a separate `UpdateKeyDescription` call
- Setting `policy` on creation previously was done once during the `CreateKey` call and again via a separate `PutKeyPolicy` call
- Setting `tags` on creation previously was done once during the `CreateKey` call and again via a separate `TagResource` call

This also adds eventual consistency retries for reading tags on resource creation and removes the resource `Exists` function, which can be another source of issues and required for the upcoming Terraform Plugin SDK v2.

Previously from operator error reports:

```
Error: error listing tags for KMS Key (***): NotFoundException: Key 'arn:aws:kms:***:key/***' does not exist

Error: error updating KMS Key (key-123) tags: error tagging resource (key-123): NotFoundException: Key 'arn:aws:kms:us-east-1:1234567890:key/key-123' does not exist
```

Output from acceptance testing:

```
--- PASS: TestAccAWSKmsKey_disappears (14.50s)
--- PASS: TestAccAWSKmsKey_asymmetricKey (40.34s)
--- PASS: TestAccAWSKmsKey_basic (43.60s)
--- PASS: TestAccAWSKmsKey_policy (58.38s)
--- PASS: TestAccAWSKmsKey_tags (59.07s)
--- PASS: TestAccAWSKmsKey_isEnabled (324.81s)
```
  • Loading branch information
bflad authored and adamdecaf committed May 28, 2020
1 parent ad15ccf commit f38d891
Showing 1 changed file with 38 additions and 35 deletions.
73 changes: 38 additions & 35 deletions aws/resource_aws_kms_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ func resourceAwsKmsKey() *schema.Resource {
Read: resourceAwsKmsKeyRead,
Update: resourceAwsKmsKeyUpdate,
Delete: resourceAwsKmsKeyDelete,
Exists: resourceAwsKmsKeyExists,

Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
Expand Down Expand Up @@ -135,7 +134,19 @@ func resourceAwsKmsKeyCreate(d *schema.ResourceData, meta interface{}) error {
d.SetId(aws.StringValue(resp.KeyMetadata.KeyId))
d.Set("key_id", resp.KeyMetadata.KeyId)

return resourceAwsKmsKeyUpdate(d, meta)
if enableKeyRotation := d.Get("enable_key_rotation").(bool); enableKeyRotation {
if err := updateKmsKeyRotationStatus(conn, d); err != nil {
return err
}
}

if enabled := d.Get("is_enabled").(bool); !enabled {
if err := updateKmsKeyStatus(conn, d.Id(), enabled); err != nil {
return err
}
}

return resourceAwsKmsKeyRead(d, meta)
}

func resourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error {
Expand Down Expand Up @@ -202,7 +213,26 @@ func resourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error {
krs, _ := out.(*kms.GetKeyRotationStatusOutput)
d.Set("enable_key_rotation", krs.KeyRotationEnabled)

tags, err := keyvaluetags.KmsListTags(conn, d.Id())
var tags keyvaluetags.KeyValueTags
err = resource.Retry(2*time.Minute, func() *resource.RetryError {
var err error
tags, err = keyvaluetags.KmsListTags(conn, d.Id())

if d.IsNewResource() && isAWSErr(err, kms.ErrCodeNotFoundException, "") {
return resource.RetryableError(err)
}

if err != nil {
return resource.NonRetryableError(err)
}

return nil
})

if isResourceTimeoutError(err) {
tags, err = keyvaluetags.KmsListTags(conn, d.Id())
}

if err != nil {
return fmt.Errorf("error listing tags for KMS Key (%s): %s", d.Id(), err)
}
Expand All @@ -217,8 +247,7 @@ func resourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error {
func resourceAwsKmsKeyUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).kmsconn

// We expect new keys to be enabled already
if d.HasChange("is_enabled") && d.Get("is_enabled").(bool) && !d.IsNewResource() {
if d.HasChange("is_enabled") && d.Get("is_enabled").(bool) {
// Enable before any attributes will be modified
if err := updateKmsKeyStatus(conn, d.Id(), d.Get("is_enabled").(bool)); err != nil {
return err
Expand Down Expand Up @@ -271,9 +300,8 @@ func resourceAwsKmsKeyDescriptionUpdate(conn *kms.KMS, d *schema.ResourceData) e
Description: aws.String(description),
KeyId: aws.String(keyId),
}
_, err := retryOnAwsCode("NotFoundException", func() (interface{}, error) {
return conn.UpdateKeyDescription(req)
})
_, err := conn.UpdateKeyDescription(req)

return err
}

Expand All @@ -291,9 +319,8 @@ func resourceAwsKmsKeyPolicyUpdate(conn *kms.KMS, d *schema.ResourceData) error
Policy: aws.String(policy),
PolicyName: aws.String("default"),
}
_, err = retryOnAwsCode("NotFoundException", func() (interface{}, error) {
return conn.PutKeyPolicy(req)
})
_, err = conn.PutKeyPolicy(req)

return err
}

Expand Down Expand Up @@ -433,30 +460,6 @@ func handleKeyRotation(conn *kms.KMS, shouldEnableRotation bool, keyId *string)
return err
}

func resourceAwsKmsKeyExists(d *schema.ResourceData, meta interface{}) (bool, error) {
conn := meta.(*AWSClient).kmsconn

req := &kms.DescribeKeyInput{
KeyId: aws.String(d.Id()),
}
resp, err := conn.DescribeKey(req)
if err != nil {
if awsErr, ok := err.(awserr.Error); ok {
if awsErr.Code() == "NotFoundException" {
return false, nil
}
}
return false, err
}
metadata := resp.KeyMetadata

if *metadata.KeyState == "PendingDeletion" {
return false, nil
}

return true, nil
}

func resourceAwsKmsKeyDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).kmsconn
keyId := d.Get("key_id").(string)
Expand Down

0 comments on commit f38d891

Please sign in to comment.