provider/aws: Enable final snapshots for aws_rds_cluster by default (#11694)

We now enable the final_snapshot of aws_rds_cluster by default. This is
a continuation of the work in #11668

```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSRDSCluster_takeFinalSnapshot'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/02/04 13:19:52 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSRDSCluster_takeFinalSnapshot -timeout 120m
=== RUN   TestAccAWSRDSCluster_takeFinalSnapshot
--- PASS: TestAccAWSRDSCluster_takeFinalSnapshot (141.59s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	141.609s
```

builtin/providers/aws/resource_aws_rds_cluster.go

@@ -114,7 +114,7 @@ func resourceAwsRDSCluster() *schema.Resource {
 			"skip_final_snapshot": {
 				Type:     schema.TypeBool,
 				Optional: true,
-				Default:  true,
+				Default:  false,
 			},
 
 			"master_username": {

builtin/providers/aws/resource_aws_rds_cluster_instance_test.go

@@ -87,41 +87,6 @@ func TestAccAWSRDSClusterInstance_disappears(t *testing.T) {
 	})
 }
 
-func testAccCheckAWSClusterInstanceDestroy(s *terraform.State) error {
-	for _, rs := range s.RootModule().Resources {
-		if rs.Type != "aws_rds_cluster" {
-			continue
-		}
-
-		// Try to find the Group
-		conn := testAccProvider.Meta().(*AWSClient).rdsconn
-		var err error
-		resp, err := conn.DescribeDBInstances(
-			&rds.DescribeDBInstancesInput{
-				DBInstanceIdentifier: aws.String(rs.Primary.ID),
-			})
-
-		if err == nil {
-			if len(resp.DBInstances) != 0 &&
-				*resp.DBInstances[0].DBInstanceIdentifier == rs.Primary.ID {
-				return fmt.Errorf("DB Cluster Instance %s still exists", rs.Primary.ID)
-			}
-		}
-
-		// Return nil if the Cluster Instance is already destroyed
-		if awsErr, ok := err.(awserr.Error); ok {
-			if awsErr.Code() == "DBInstanceNotFound" {
-				return nil
-			}
-		}
-
-		return err
-
-	}
-
-	return nil
-}
-
 func testAccCheckAWSDBClusterInstanceAttributes(v *rds.DBInstance) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 
@@ -224,6 +189,7 @@ resource "aws_rds_cluster" "default" {
   database_name      = "mydb"
   master_username    = "foo"
   master_password    = "mustbeeightcharaters"
+  skip_final_snapshot = true
 }
 
 resource "aws_rds_cluster_instance" "cluster_instances" {
@@ -259,6 +225,7 @@ resource "aws_rds_cluster" "default" {
   database_name      = "mydb"
   master_username    = "foo"
   master_password    = "mustbeeightcharaters"
+  skip_final_snapshot = true
 }
 
 resource "aws_rds_cluster_instance" "cluster_instances" {
@@ -319,6 +286,7 @@ resource "aws_rds_cluster" "default" {
   master_password    = "mustbeeightcharaters"
   storage_encrypted = true
   kms_key_id = "${aws_kms_key.foo.arn}"
+  skip_final_snapshot = true
 }
 
 resource "aws_rds_cluster_instance" "cluster_instances" {
@@ -353,6 +321,7 @@ resource "aws_rds_cluster" "default" {
   database_name      = "mydb"
   master_username    = "foo"
   master_password    = "mustbeeightcharaters"
+  skip_final_snapshot = true
 }
 
 resource "aws_rds_cluster_instance" "cluster_instances" {

builtin/providers/aws/resource_aws_rds_cluster_test.go

@@ -2,7 +2,9 @@ package aws
 
 import (
 	"fmt"
+	"log"
 	"regexp"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/acctest"
@@ -38,6 +40,25 @@ func TestAccAWSRDSCluster_basic(t *testing.T) {
 	})
 }
 
+func TestAccAWSRDSCluster_takeFinalSnapshot(t *testing.T) {
+	var v rds.DBCluster
+	rInt := acctest.RandInt()
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSClusterSnapshot(rInt),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSClusterConfigWithFinalSnapshot(rInt),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSClusterExists("aws_rds_cluster.default", &v),
+				),
+			},
+		},
+	})
+}
+
 /// This is a regression test to make sure that we always cover the scenario as hightlighted in
 /// https://github.com/hashicorp/terraform/issues/11568
 func TestAccAWSRDSCluster_missingUserNameCausesError(t *testing.T) {
@@ -198,6 +219,61 @@ func testAccCheckAWSClusterDestroy(s *terraform.State) error {
 	return nil
 }
 
+func testAccCheckAWSClusterSnapshot(rInt int) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		for _, rs := range s.RootModule().Resources {
+			if rs.Type != "aws_rds_cluster" {
+				continue
+			}
+
+			snapshot_identifier := fmt.Sprintf("foobarbaz-test-terraform-final-snapshot-%d", rInt)
+
+			// Try to find the Group
+			conn := testAccProvider.Meta().(*AWSClient).rdsconn
+			var err error
+			resp, err := conn.DescribeDBClusters(
+				&rds.DescribeDBClustersInput{
+					DBClusterIdentifier: aws.String(rs.Primary.ID),
+				})
+
+			if err == nil {
+				if len(resp.DBClusters) != 0 &&
+					*resp.DBClusters[0].DBClusterIdentifier == rs.Primary.ID {
+					return fmt.Errorf("DB Cluster %s still exists", rs.Primary.ID)
+				}
+			}
+
+			// Return nil if the cluster is already destroyed
+			if awsErr, ok := err.(awserr.Error); ok {
+				if awsErr.Code() == "DBClusterNotFoundFault" {
+					return nil
+				}
+			} else {
+				awsClient := testAccProvider.Meta().(*AWSClient)
+				arn, err := buildRDSClusterARN(snapshot_identifier, awsClient.partition, awsClient.accountid, awsClient.region)
+				tagsARN := strings.Replace(arn, ":cluster:", ":snapshot:", 1)
+				if err != nil {
+					return fmt.Errorf("Error building ARN for tags check with ARN (%s): %s", tagsARN, err)
+				}
+
+				log.Printf("[INFO] Deleting the Snapshot %s", snapshot_identifier)
+				_, snapDeleteErr := conn.DeleteDBClusterSnapshot(
+					&rds.DeleteDBClusterSnapshotInput{
+						DBClusterSnapshotIdentifier: aws.String(snapshot_identifier),
+					})
+				if snapDeleteErr != nil {
+					return err
+				}
+
+			}
+
+			return err
+		}
+
+		return nil
+	}
+}
+
 func testAccCheckAWSClusterExists(n string, v *rds.DBCluster) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
@@ -238,18 +314,37 @@ resource "aws_rds_cluster" "default" {
   master_username = "foo"
   master_password = "mustbeeightcharaters"
   db_cluster_parameter_group_name = "default.aurora5.6"
+  skip_final_snapshot = true
   tags {
     Environment = "production"
   }
 }`, n)
 }
 
+func testAccAWSClusterConfigWithFinalSnapshot(n int) string {
+	return fmt.Sprintf(`
+resource "aws_rds_cluster" "default" {
+  cluster_identifier = "tf-aurora-cluster-%d"
+  availability_zones = ["us-west-2a","us-west-2b","us-west-2c"]
+  database_name = "mydb"
+  master_username = "foo"
+  master_password = "mustbeeightcharaters"
+  db_cluster_parameter_group_name = "default.aurora5.6"
+  skip_final_snapshot = true
+  final_snapshot_identifier = "tf-acctest-rdscluster-snapshot-%d"
+  tags {
+    Environment = "production"
+  }
+}`, n, n)
+}
+
 func testAccAWSClusterConfigWithoutUserNameAndPassword(n int) string {
 	return fmt.Sprintf(`
 resource "aws_rds_cluster" "default" {
   cluster_identifier = "tf-aurora-cluster-%d"
   availability_zones = ["us-west-2a","us-west-2b","us-west-2c"]
   database_name = "mydb"
+  skip_final_snapshot = true
 }`, n)
 }
 
@@ -262,6 +357,7 @@ resource "aws_rds_cluster" "default" {
   master_username = "foo"
   master_password = "mustbeeightcharaters"
   db_cluster_parameter_group_name = "default.aurora5.6"
+  skip_final_snapshot = true
   tags {
     Environment = "production"
     AnotherTag = "test"
@@ -302,6 +398,7 @@ func testAccAWSClusterConfig_kmsKey(n int) string {
    db_cluster_parameter_group_name = "default.aurora5.6"
    storage_encrypted = true
    kms_key_id = "${aws_kms_key.foo.arn}"
+   skip_final_snapshot = true
  }`, n, n)
 }
 
@@ -314,6 +411,7 @@ resource "aws_rds_cluster" "default" {
   master_username = "foo"
   master_password = "mustbeeightcharaters"
   storage_encrypted = true
+  skip_final_snapshot = true
 }`, n)
 }
 
@@ -328,6 +426,7 @@ resource "aws_rds_cluster" "default" {
   backup_retention_period = 5
   preferred_backup_window = "07:00-09:00"
   preferred_maintenance_window = "tue:04:00-tue:04:30"
+  skip_final_snapshot = true
 }`, n)
 }
 
@@ -343,5 +442,6 @@ resource "aws_rds_cluster" "default" {
   preferred_backup_window = "03:00-09:00"
   preferred_maintenance_window = "wed:01:00-wed:01:30"
   apply_immediately = true
+  skip_final_snapshot = true
 }`, n)
 }

website/source/docs/providers/aws/r/rds_cluster.html.markdown

@@ -61,7 +61,7 @@ string.
 * `final_snapshot_identifier` - (Optional) The name of your final DB snapshot
     when this DB cluster is deleted. If omitted, no final snapshot will be
     made.
-* `skip_final_snapshot` - (Optional) Determines whether a final DB snapshot is created before the DB cluster is deleted. If true is specified, no DB snapshot is created. If false is specified, a DB snapshot is created before the DB cluster is deleted, using the value from `final_snapshot_identifier`. Default is true.
+* `skip_final_snapshot` - (Optional) Determines whether a final DB snapshot is created before the DB cluster is deleted. If true is specified, no DB snapshot is created. If false is specified, a DB snapshot is created before the DB cluster is deleted, using the value from `final_snapshot_identifier`. Default is `false`.
 * `availability_zones` - (Optional) A list of EC2 Availability Zones that
   instances in the DB cluster can be created in
 * `backup_retention_period` - (Optional) The days to retain backups for. Default