Skip to content

Commit

Permalink
Merge pull request #9415 from terraform-providers/rfd-retry-kms
Browse files Browse the repository at this point in the history 
Retries after timeouts for KMS keys and grants.
  • Loading branch information
@ryndaniels
ryndaniels authored Jul 24, 2019
2 parents 47a6e20 + fcc957f commit 3077dfb
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 37 deletions.
21 changes: 18 additions & 3 deletions aws/resource_aws_kms_grant.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ func resourceAwsKmsGrantCreate(d *schema.ResourceData, meta interface{}) error {

err := resource.Retry(3*time.Minute, func() *resource.RetryError {
var err error

out, err = conn.CreateGrant(&input)

if err != nil {
Expand All @@ -169,8 +168,12 @@ func resourceAwsKmsGrantCreate(d *schema.ResourceData, meta interface{}) error {
return nil
})

if isResourceTimeoutError(err) {
out, err = conn.CreateGrant(&input)
}

if err != nil {
return err
return fmt.Errorf("Error creating KMS grant: %s", err)
}

log.Printf("[DEBUG] Created new KMS Grant: %s", *out.GrantId)
Expand Down Expand Up @@ -333,14 +336,19 @@ func findKmsGrantByIdWithRetry(conn *kms.KMS, keyId string, grantId string) (*km

return nil
})
if isResourceTimeoutError(err) {
grant, err = findKmsGrantById(conn, keyId, grantId, nil)
}

return grant, err
}

// Used by the tests as well
func waitForKmsGrantToBeRevoked(conn *kms.KMS, keyId string, grantId string) error {
var grant *kms.GrantListEntry
err := resource.Retry(3*time.Minute, func() *resource.RetryError {
grant, err := findKmsGrantById(conn, keyId, grantId, nil)
var err error
grant, err = findKmsGrantById(conn, keyId, grantId, nil)

if isResourceNotFoundError(err) {
return nil
Expand All @@ -354,6 +362,9 @@ func waitForKmsGrantToBeRevoked(conn *kms.KMS, keyId string, grantId string) err

return resource.NonRetryableError(err)
})
if isResourceTimeoutError(err) {
grant, err = findKmsGrantById(conn, keyId, grantId, nil)
}

return err
}
Expand Down Expand Up @@ -387,6 +398,10 @@ func findKmsGrantById(conn *kms.KMS, keyId string, grantId string, marker *strin

return nil
})
if isResourceTimeoutError(err) {
out, err = conn.ListGrants(&input)
}

if err != nil {
return nil, fmt.Errorf("error listing KMS Grants: %s", err)
}
Expand Down
45 changes: 23 additions & 22 deletions aws/resource_aws_kms_grant_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"
"time"

"github.com/hashicorp/terraform/helper/acctest"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
)
Expand Down Expand Up @@ -169,18 +170,18 @@ resource "aws_kms_key" "tf-acc-test-key" {
%s
resource "aws_iam_role" "tf-acc-test-role" {
name = "tf-acc-test-kms-grant-role-%s"
name = "%s"
path = "/service-role/"
assume_role_policy = "${data.aws_iam_policy_document.assumerole-policy-template.json}"
}
resource "aws_kms_grant" "%s" {
name = "%s"
resource "aws_kms_grant" "%[4]s" {
name = "%[4]s"
key_id = "${aws_kms_key.tf-acc-test-key.key_id}"
grantee_principal = "${aws_iam_role.tf-acc-test-role.arn}"
operations = [ %s ]
operations = [ %[5]s ]
}
`, timestamp, staticAssumeRolePolicyString, rName, rName, rName, operations)
`, timestamp, staticAssumeRolePolicyString, acctest.RandomWithPrefix("tf-acc-test-kms-grant-role"), rName, operations)
}

func testAccAWSKmsGrant_withConstraints(rName string, timestamp string, constraintName string, encryptionContext string) string {
Expand All @@ -193,23 +194,23 @@ resource "aws_kms_key" "tf-acc-test-key" {
%s
resource "aws_iam_role" "tf-acc-test-role" {
name = "tf-acc-test-kms-grant-role-%s"
name = "%s"
path = "/service-role/"
assume_role_policy = "${data.aws_iam_policy_document.assumerole-policy-template.json}"
}
resource "aws_kms_grant" "%s" {
name = "%s"
resource "aws_kms_grant" "%[4]s" {
name = "%[4]s"
key_id = "${aws_kms_key.tf-acc-test-key.key_id}"
grantee_principal = "${aws_iam_role.tf-acc-test-role.arn}"
operations = [ "RetireGrant", "DescribeKey" ]
constraints {
%s = {
%s
%[5]s = {
%[6]s
}
}
}
`, timestamp, staticAssumeRolePolicyString, rName, rName, rName, constraintName, encryptionContext)
`, timestamp, staticAssumeRolePolicyString, acctest.RandomWithPrefix("tf-acc-test-kms-grant-role"), rName, constraintName, encryptionContext)
}

func testAccAWSKmsGrant_withRetiringPrincipal(rName string, timestamp string) string {
Expand All @@ -222,19 +223,19 @@ resource "aws_kms_key" "tf-acc-test-key" {
%s
resource "aws_iam_role" "tf-acc-test-role" {
name = "tf-acc-test-kms-grant-role-%s"
name = "%s"
path = "/service-role/"
assume_role_policy = "${data.aws_iam_policy_document.assumerole-policy-template.json}"
}
resource "aws_kms_grant" "%s" {
name = "%s"
resource "aws_kms_grant" "%[4]s" {
name = "%[4]s"
key_id = "${aws_kms_key.tf-acc-test-key.key_id}"
grantee_principal = "${aws_iam_role.tf-acc-test-role.arn}"
operations = ["ReEncryptTo", "CreateGrant"]
retiring_principal = "${aws_iam_role.tf-acc-test-role.arn}"
}
`, timestamp, staticAssumeRolePolicyString, rName, rName, rName)
`, timestamp, staticAssumeRolePolicyString, acctest.RandomWithPrefix("tf-acc-test-kms-grant-role"), rName)
}

func testAccAWSKmsGrant_bare(rName string, timestamp string) string {
Expand All @@ -247,7 +248,7 @@ resource "aws_kms_key" "tf-acc-test-key" {
%s
resource "aws_iam_role" "tf-acc-test-role" {
name = "tf-acc-test-kms-grant-role-%s"
name = "%s"
path = "/service-role/"
assume_role_policy = "${data.aws_iam_policy_document.assumerole-policy-template.json}"
}
Expand All @@ -257,7 +258,7 @@ resource "aws_kms_grant" "%s" {
grantee_principal = "${aws_iam_role.tf-acc-test-role.arn}"
operations = ["ReEncryptTo", "CreateGrant"]
}
`, timestamp, staticAssumeRolePolicyString, rName, rName)
`, timestamp, staticAssumeRolePolicyString, acctest.RandomWithPrefix("tf-acc-test-kms-grant-role"), rName)
}

const staticAssumeRolePolicyString = `
Expand All @@ -283,16 +284,16 @@ resource "aws_kms_key" "tf-acc-test-key" {
%s
resource "aws_iam_role" "tf-acc-test-role" {
name = "tf-acc-test-kms-grant-role-%s"
name = "%s"
path = "/service-role/"
assume_role_policy = "${data.aws_iam_policy_document.assumerole-policy-template.json}"
}
resource "aws_kms_grant" "%s" {
name = "%s"
resource "aws_kms_grant" "%[4]s" {
name = "%[4]s"
key_id = "${aws_kms_key.tf-acc-test-key.arn}"
grantee_principal = "${aws_iam_role.tf-acc-test-role.arn}"
operations = [ %s ]
operations = [ %[5]s ]
}
`, timestamp, staticAssumeRolePolicyString, rName, rName, rName, operations)
`, timestamp, staticAssumeRolePolicyString, acctest.RandomWithPrefix("tf-acc-test-kms-grant-role"), rName, operations)
}
35 changes: 23 additions & 12 deletions aws/resource_aws_kms_key.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ func resourceAwsKmsKeyCreate(d *schema.ResourceData, meta interface{}) error {
}
return resource.NonRetryableError(err)
})
if isResourceTimeoutError(err) {
resp, err = conn.CreateKey(&req)
}
if err != nil {
return err
}
Expand Down Expand Up @@ -335,18 +338,7 @@ func updateKmsKeyRotationStatus(conn *kms.KMS, d *schema.ResourceData) error {
shouldEnableRotation := d.Get("enable_key_rotation").(bool)

err := resource.Retry(10*time.Minute, func() *resource.RetryError {
var err error
if shouldEnableRotation {
log.Printf("[DEBUG] Enabling key rotation for KMS key %q", d.Id())
_, err = conn.EnableKeyRotation(&kms.EnableKeyRotationInput{
KeyId: aws.String(d.Id()),
})
} else {
log.Printf("[DEBUG] Disabling key rotation for KMS key %q", d.Id())
_, err = conn.DisableKeyRotation(&kms.DisableKeyRotationInput{
KeyId: aws.String(d.Id()),
})
}
err := handleKeyRotation(conn, shouldEnableRotation, aws.String(d.Id()))

if err != nil {
awsErr, ok := err.(awserr.Error)
Expand All @@ -362,6 +354,9 @@ func updateKmsKeyRotationStatus(conn *kms.KMS, d *schema.ResourceData) error {

return nil
})
if isResourceTimeoutError(err) {
err = handleKeyRotation(conn, shouldEnableRotation, aws.String(d.Id()))
}

if err != nil {
return fmt.Errorf("Failed to set key rotation for %q to %t: %q",
Expand Down Expand Up @@ -404,6 +399,22 @@ func updateKmsKeyRotationStatus(conn *kms.KMS, d *schema.ResourceData) error {
return nil
}

func handleKeyRotation(conn *kms.KMS, shouldEnableRotation bool, keyId *string) error {
var err error
if shouldEnableRotation {
log.Printf("[DEBUG] Enabling key rotation for KMS key %q", *keyId)
_, err = conn.EnableKeyRotation(&kms.EnableKeyRotationInput{
KeyId: keyId,
})
} else {
log.Printf("[DEBUG] Disabling key rotation for KMS key %q", *keyId)
_, err = conn.DisableKeyRotation(&kms.DisableKeyRotationInput{
KeyId: keyId,
})
}
return err
}

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

Expand Down

0 comments on commit 3077dfb

Please sign in to comment.