Skip to content

Commit

Permalink
resource/aws_db_instance: Ensure monitoring attributes are always wri…
Browse files Browse the repository at this point in the history 
…tten to state and retry ModifyDBInstance on IAM eventual consistency error

References:

- #315
- #2188
- #5559

Previously before code updates:

```
--- FAIL: TestAccAWSDBInstance_MonitoringRoleArn_RemovedToEnabled (430.95s)
    testing.go:568: Step 2 error: errors during apply:

        Error: Error modifying DB Instance tf-acc-test-1165998526456666486: InvalidParameterValue: IAM role ARN value is invalid or does not include the required permissions for: ENHANCED_MONITORING
        	status code: 400, request id: 524f599d-3870-48b3-843e-28885ae3f75c

          on /var/folders/v0/_d108fkx1pbbg4_sh864_7740000gn/T/tf-test908490254/main.tf line 29:
          (source code not available)

--- PASS: TestAccAWSDBInstance_MonitoringRoleInterval (565.47s)
--- PASS: TestAccAWSDBInstance_MonitoringRoleArn_EnabledToDisabled (593.87s)
--- PASS: TestAccAWSDBInstance_MonitoringRoleArn_EnabledToRemoved (626.12s)
```

Output from acceptance testing:

```
--- PASS: TestAccAWSDBInstance_MonitoringRoleArn_EnabledToDisabled (587.93s)
--- PASS: TestAccAWSDBInstance_MonitoringRoleArn_RemovedToEnabled (614.89s)
--- PASS: TestAccAWSDBInstance_MonitoringRoleArn_EnabledToRemoved (656.13s)
--- PASS: TestAccAWSDBInstance_MonitoringRoleInterval (702.57s)
```
  • Loading branch information
@bflad
bflad committed Aug 13, 2019
1 parent a41a63a commit d9cac8a
Show file tree
Hide file tree
Showing 2 changed files with 240 additions and 71 deletions.
30 changes: 22 additions & 8 deletions aws/resource_aws_db_instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1320,13 +1320,8 @@ func resourceAwsDbInstanceRead(d *schema.ResourceData, meta interface{}) error {
d.Set("option_group_name", v.OptionGroupMemberships[0].OptionGroupName)
}

if v.MonitoringInterval != nil {
d.Set("monitoring_interval", v.MonitoringInterval)
}

if v.MonitoringRoleArn != nil {
d.Set("monitoring_role_arn", v.MonitoringRoleArn)
}
d.Set("monitoring_interval", v.MonitoringInterval)
d.Set("monitoring_role_arn", v.MonitoringRoleArn)

if err := d.Set("enabled_cloudwatch_logs_exports", flattenStringList(v.EnabledCloudwatchLogsExports)); err != nil {
return fmt.Errorf("error setting enabled_cloudwatch_logs_exports: %s", err)
Expand Down Expand Up @@ -1641,7 +1636,26 @@ func resourceAwsDbInstanceUpdate(d *schema.ResourceData, meta interface{}) error
log.Printf("[DEBUG] Send DB Instance Modification request: %t", requestUpdate)
if requestUpdate {
log.Printf("[DEBUG] DB Instance Modification request: %s", req)
_, err := conn.ModifyDBInstance(req)

err := resource.Retry(2*time.Minute, func() *resource.RetryError {
_, err := conn.ModifyDBInstance(req)

// Retry for IAM eventual consistency
if isAWSErr(err, "InvalidParameterValue", "IAM role ARN value is invalid or does not include the required permissions") {
return resource.RetryableError(err)
}

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

return nil
})

if isResourceTimeoutError(err) {
_, err = conn.ModifyDBInstance(req)
}

if err != nil {
return fmt.Errorf("Error modifying DB Instance %s: %s", d.Id(), err)
}
Expand Down
281 changes: 218 additions & 63 deletions aws/resource_aws_db_instance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1555,21 +1555,157 @@ func TestAccAWSDBInstance_SnapshotIdentifier_VpcSecurityGroupIds_Tags(t *testing
})
}

func TestAccAWSDBInstance_enhancedMonitoring(t *testing.T) {
func TestAccAWSDBInstance_MonitoringRoleInterval(t *testing.T) {
var dbInstance rds.DBInstance
rName := acctest.RandString(5)
resourceName := "aws_db_instance.test"
rName := acctest.RandomWithPrefix("tf-acc-test")

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSDBInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccSnapshotInstanceConfig_enhancedMonitoring(rName),
Config: testAccDbInstanceConfigMonitoringInterval(rName, 30),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBInstanceExists("aws_db_instance.enhanced_monitoring", &dbInstance),
resource.TestCheckResourceAttr(
"aws_db_instance.enhanced_monitoring", "monitoring_interval", "5"),
testAccCheckAWSDBInstanceExists(resourceName, &dbInstance),
resource.TestCheckResourceAttr(resourceName, "monitoring_interval", "30"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{
"apply_immediately",
"final_snapshot_identifier",
"password",
"skip_final_snapshot",
},
},
{
Config: testAccDbInstanceConfigMonitoringInterval(rName, 60),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBInstanceExists(resourceName, &dbInstance),
resource.TestCheckResourceAttr(resourceName, "monitoring_interval", "60"),
),
},
},
})
}

func TestAccAWSDBInstance_MonitoringRoleArn_EnabledToDisabled(t *testing.T) {
var dbInstance rds.DBInstance
iamRoleResourceName := "aws_iam_role.test"
resourceName := "aws_db_instance.test"
rName := acctest.RandomWithPrefix("tf-acc-test")

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSDBInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccDbInstanceConfigMonitoringRoleArn(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBInstanceExists(resourceName, &dbInstance),
resource.TestCheckResourceAttrPair(resourceName, "monitoring_role_arn", iamRoleResourceName, "arn"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{
"apply_immediately",
"final_snapshot_identifier",
"password",
"skip_final_snapshot",
},
},
{
Config: testAccDbInstanceConfigMonitoringInterval(rName, 0),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBInstanceExists(resourceName, &dbInstance),
resource.TestCheckResourceAttr(resourceName, "monitoring_interval", "0"),
),
},
},
})
}

func TestAccAWSDBInstance_MonitoringRoleArn_EnabledToRemoved(t *testing.T) {
var dbInstance rds.DBInstance
iamRoleResourceName := "aws_iam_role.test"
resourceName := "aws_db_instance.test"
rName := acctest.RandomWithPrefix("tf-acc-test")

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSDBInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccDbInstanceConfigMonitoringRoleArn(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBInstanceExists(resourceName, &dbInstance),
resource.TestCheckResourceAttrPair(resourceName, "monitoring_role_arn", iamRoleResourceName, "arn"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{
"apply_immediately",
"final_snapshot_identifier",
"password",
"skip_final_snapshot",
},
},
{
Config: testAccDbInstanceConfigMonitoringRoleArnRemoved(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBInstanceExists(resourceName, &dbInstance),
),
},
},
})
}

func TestAccAWSDBInstance_MonitoringRoleArn_RemovedToEnabled(t *testing.T) {
var dbInstance rds.DBInstance
iamRoleResourceName := "aws_iam_role.test"
resourceName := "aws_db_instance.test"
rName := acctest.RandomWithPrefix("tf-acc-test")

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSDBInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccDbInstanceConfigMonitoringRoleArnRemoved(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBInstanceExists(resourceName, &dbInstance),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{
"apply_immediately",
"final_snapshot_identifier",
"password",
"skip_final_snapshot",
},
},
{
Config: testAccDbInstanceConfigMonitoringRoleArn(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBInstanceExists(resourceName, &dbInstance),
resource.TestCheckResourceAttrPair(resourceName, "monitoring_role_arn", iamRoleResourceName, "arn"),
),
},
},
Expand Down Expand Up @@ -2690,10 +2826,12 @@ resource "aws_db_instance" "snapshot" {
`, rInt, rInt)
}

func testAccSnapshotInstanceConfig_enhancedMonitoring(rName string) string {
func testAccDbInstanceConfigMonitoringInterval(rName string, monitoringInterval int) string {
return fmt.Sprintf(`
resource "aws_iam_role" "enhanced_policy_role" {
name = "enhanced-monitoring-role-%s"
data "aws_partition" "current" {}
resource "aws_iam_role" "test" {
name = %[1]q
assume_role_policy = <<EOF
{
Expand All @@ -2712,73 +2850,90 @@ resource "aws_iam_role" "enhanced_policy_role" {
EOF
}
resource "aws_iam_policy_attachment" "test-attach" {
name = "enhanced-monitoring-attachment-%s"
resource "aws_iam_role_policy_attachment" "test" {
policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/service-role/AmazonRDSEnhancedMonitoringRole"
role = "${aws_iam_role.test.name}"
}
roles = [
"${aws_iam_role.enhanced_policy_role.name}",
]
resource "aws_db_instance" "test" {
depends_on = ["aws_iam_role_policy_attachment.test"]
policy_arn = "${aws_iam_policy.test.arn}"
allocated_storage = 5
engine = "mysql"
engine_version = "5.6.35"
identifier = %[1]q
instance_class = "db.t2.micro"
monitoring_interval = %[2]d
monitoring_role_arn = "${aws_iam_role.test.arn}"
name = "baz"
password = "barbarbarbar"
skip_final_snapshot = true
username = "foo"
}
`, rName, monitoringInterval)
}

resource "aws_iam_policy" "test" {
name = "tf-enhanced-monitoring-policy-%s"
policy = <<POLICY
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "EnableCreationAndManagementOfRDSCloudwatchLogGroups",
"Effect": "Allow",
"Action": [
"logs:CreateLogGroup",
"logs:PutRetentionPolicy"
],
"Resource": [
"arn:aws:logs:*:*:log-group:RDS*"
]
},
{
"Sid": "EnableCreationAndManagementOfRDSCloudwatchLogStreams",
"Effect": "Allow",
"Action": [
"logs:CreateLogStream",
"logs:PutLogEvents",
"logs:DescribeLogStreams",
"logs:GetLogEvents"
],
"Resource": [
"arn:aws:logs:*:*:log-group:RDS*:log-stream:*"
]
}
]
func testAccDbInstanceConfigMonitoringRoleArnRemoved(rName string) string {
return fmt.Sprintf(`
resource "aws_db_instance" "test" {
allocated_storage = 5
engine = "mysql"
engine_version = "5.6.35"
identifier = %[1]q
instance_class = "db.t2.micro"
name = "baz"
password = "barbarbarbar"
skip_final_snapshot = true
username = "foo"
}
POLICY
`, rName)
}

resource "aws_db_instance" "enhanced_monitoring" {
identifier = "foobarbaz-enhanced-monitoring-%s"
depends_on = ["aws_iam_policy_attachment.test-attach"]
func testAccDbInstanceConfigMonitoringRoleArn(rName string) string {
return fmt.Sprintf(`
data "aws_partition" "current" {}
allocated_storage = 5
engine = "mysql"
engine_version = "5.6.35"
instance_class = "db.t2.micro"
name = "baz"
password = "barbarbarbar"
username = "foo"
backup_retention_period = 1
resource "aws_iam_role" "test" {
name = %[1]q
parameter_group_name = "default.mysql5.6"
assume_role_policy = <<EOF
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "",
"Effect": "Allow",
"Principal": {
"Service": "monitoring.rds.amazonaws.com"
},
"Action": "sts:AssumeRole"
}
]
}
EOF
}
monitoring_role_arn = "${aws_iam_role.enhanced_policy_role.arn}"
monitoring_interval = "5"
resource "aws_iam_role_policy_attachment" "test" {
policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/service-role/AmazonRDSEnhancedMonitoringRole"
role = "${aws_iam_role.test.name}"
}
resource "aws_db_instance" "test" {
depends_on = ["aws_iam_role_policy_attachment.test"]
allocated_storage = 5
engine = "mysql"
engine_version = "5.6.35"
identifier = %[1]q
instance_class = "db.t2.micro"
monitoring_interval = 5
monitoring_role_arn = "${aws_iam_role.test.arn}"
name = "baz"
password = "barbarbarbar"
skip_final_snapshot = true
username = "foo"
}
`, rName, rName, rName, rName)
`, rName)
}

func testAccSnapshotInstanceConfig_iopsUpdate(rName string, iops int) string {
Expand Down

0 comments on commit d9cac8a

Please sign in to comment.