Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retries after timeouts for KMS keys and grants. #9415

Merged
merged 2 commits into from
Jul 24, 2019
Merged

Retries after timeouts for KMS keys and grants. #9415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec34f3f
Retries after timeouts for KMS keys and grants
ryndaniels Jul 19, 2019
fcc957f
lint for the linting throne
ryndaniels Jul 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions aws/resource_aws_kms_grant.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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