From 0ef55ba95d5385222f265902305553908feaf606 Mon Sep 17 00:00:00 2001 From: pvanbuijtene Date: Mon, 30 Jul 2018 21:11:29 +0200 Subject: [PATCH 1/2] Fix IAM eventual consistency error when creating db_option_group --- aws/resource_aws_db_option_group.go | 16 ++++- aws/resource_aws_db_option_group_test.go | 84 ++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_db_option_group.go b/aws/resource_aws_db_option_group.go index 28f68b8d188..294d6a536ff 100644 --- a/aws/resource_aws_db_option_group.go +++ b/aws/resource_aws_db_option_group.go @@ -273,12 +273,24 @@ func resourceAwsDbOptionGroupUpdate(d *schema.ResourceData, meta interface{}) er } log.Printf("[DEBUG] Modify DB Option Group: %s", modifyOpts) - _, err = rdsconn.ModifyOptionGroup(modifyOpts) + + err = resource.Retry(5*time.Minute, func() *resource.RetryError { + var err error + + _, err = rdsconn.ModifyOptionGroup(modifyOpts) + if err != nil { + if isAWSErr(err, "InvalidParameterValue", "SQLSERVER_BACKUP_RESTORE") { + return resource.RetryableError(err) + } + return resource.NonRetryableError(err) + } + return nil + }) + if err != nil { return fmt.Errorf("Error modifying DB Option Group: %s", err) } d.SetPartial("option") - } if err := setTagsRDS(rdsconn, d, d.Get("arn").(string)); err != nil { diff --git a/aws/resource_aws_db_option_group_test.go b/aws/resource_aws_db_option_group_test.go index d24e193cba4..5ad41d1236f 100644 --- a/aws/resource_aws_db_option_group_test.go +++ b/aws/resource_aws_db_option_group_test.go @@ -233,6 +233,30 @@ func TestAccAWSDBOptionGroup_OptionSettings(t *testing.T) { }) } +func TestAccAWSDBOptionGroup_OptionSettingsIAMRole(t *testing.T) { + var v rds.OptionGroup + rName := fmt.Sprintf("option-group-test-terraform-%s", acctest.RandString(5)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDBOptionGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSDBOptionGroupOptionSettingsIAMRole(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBOptionGroupExists("aws_db_option_group.bar", &v), + resource.TestCheckResourceAttr( + "aws_db_option_group.bar", "name", rName), + resource.TestCheckResourceAttr( + "aws_db_option_group.bar", "option.#", "1"), + testAccCheckAWSDBOptionGroupOptionSettingsIAMRole(&v), + ), + }, + }, + }) +} + func TestAccAWSDBOptionGroup_sqlServerOptionsUpdate(t *testing.T) { var v rds.OptionGroup rName := fmt.Sprintf("option-group-test-terraform-%s", acctest.RandString(5)) @@ -343,6 +367,32 @@ func testAccCheckAWSDBOptionGroupAttributes(v *rds.OptionGroup) resource.TestChe } } +func testAccCheckAWSDBOptionGroupOptionSettingsIAMRole(optionGroup *rds.OptionGroup) resource.TestCheckFunc { + return func(s *terraform.State) error { + if optionGroup == nil { + return errors.New("Option Group does not exist") + } + if len(optionGroup.Options) == 0 { + return errors.New("Option Group does not have any options") + } + if len(optionGroup.Options[0].OptionSettings) == 0 { + return errors.New("Option Group does not have any option settings") + } + + settingName := aws.StringValue(optionGroup.Options[0].OptionSettings[0].Name) + if settingName != "IAM_ROLE_ARN" { + return fmt.Errorf("Expected option setting IAM_ROLE_ARN and received %s", settingName) + } + + settingValue := aws.StringValue(optionGroup.Options[0].OptionSettings[0].Value) + iamArnRegExp := regexp.MustCompile(`^arn:aws:iam::\d{12}:role/.+`) + if !iamArnRegExp.MatchString(settingValue) { + return fmt.Errorf("Expected option setting to be a valid IAM role but received %s", settingValue) + } + return nil + } +} + func testAccCheckAWSDBOptionGroupOptionVersionAttribute(optionGroup *rds.OptionGroup, optionVersion string) resource.TestCheckFunc { return func(s *terraform.State) error { if optionGroup == nil { @@ -499,6 +549,40 @@ resource "aws_db_option_group" "bar" { `, r) } +func testAccAWSDBOptionGroupOptionSettingsIAMRole(r string) string { + return fmt.Sprintf(` +data "aws_iam_policy_document" "rds_assume_role" { + statement { + actions = ["sts:AssumeRole"] + principals { + type = "Service" + identifiers = ["rds.amazonaws.com"] + } + } +} + +resource "aws_iam_role" "sql_server_backup" { + name = "rds-backup-%s" + assume_role_policy = "${data.aws_iam_policy_document.rds_assume_role.json}" +} + +resource "aws_db_option_group" "bar" { + name = "%s" + option_group_description = "Test option group for terraform" + engine_name = "sqlserver-ex" + major_engine_version = "14.00" + + option { + option_name = "SQLSERVER_BACKUP_RESTORE" + option_settings { + name = "IAM_ROLE_ARN" + value = "${aws_iam_role.sql_server_backup.arn}" + } + } +} +`, r, r) +} + func testAccAWSDBOptionGroupOptionSettings_update(r string) string { return fmt.Sprintf(` resource "aws_db_option_group" "bar" { From 4f1048898eec52e07b9c807c06dd294b2b9ef10b Mon Sep 17 00:00:00 2001 From: pvanbuijtene Date: Mon, 30 Jul 2018 22:53:39 +0200 Subject: [PATCH 2/2] Apply review comments --- aws/resource_aws_db_option_group.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_db_option_group.go b/aws/resource_aws_db_option_group.go index 294d6a536ff..cc535f5a040 100644 --- a/aws/resource_aws_db_option_group.go +++ b/aws/resource_aws_db_option_group.go @@ -274,12 +274,13 @@ func resourceAwsDbOptionGroupUpdate(d *schema.ResourceData, meta interface{}) er log.Printf("[DEBUG] Modify DB Option Group: %s", modifyOpts) - err = resource.Retry(5*time.Minute, func() *resource.RetryError { + err = resource.Retry(2*time.Minute, func() *resource.RetryError { var err error _, err = rdsconn.ModifyOptionGroup(modifyOpts) if err != nil { - if isAWSErr(err, "InvalidParameterValue", "SQLSERVER_BACKUP_RESTORE") { + // InvalidParameterValue: IAM role ARN value is invalid or does not include the required permissions for: SQLSERVER_BACKUP_RESTORE + if isAWSErr(err, "InvalidParameterValue", "IAM role ARN value is invalid or does not include the required permissions") { return resource.RetryableError(err) } return resource.NonRetryableError(err)