provider/aws: Set aws_db_cluster to snapshot by default (#11668)

As requested in #11448

Terraform didn't snapshot AWS DB Instances by default. We are going to
change that behaviour going forward.

builtin/providers/aws/resource_aws_db_instance.go

@@ -207,7 +207,7 @@ func resourceAwsDbInstance() *schema.Resource {
 			"skip_final_snapshot": {
 				Type:     schema.TypeBool,
 				Optional: true,
-				Default:  true,
+				Default:  false,
 			},
 
 			"copy_tags_to_snapshot": {

builtin/providers/aws/resource_aws_db_instance_test.go

@@ -150,15 +150,13 @@ func TestAccAWSDBInstanceReplica(t *testing.T) {
 	})
 }
 
-func TestAccAWSDBInstanceSnapshot(t *testing.T) {
+func TestAccAWSDBInstanceNoSnapshot(t *testing.T) {
 	var snap rds.DBInstance
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:  func() { testAccPreCheck(t) },
-		Providers: testAccProviders,
-		// testAccCheckAWSDBInstanceSnapshot verifies a database snapshot is
-		// created, and subequently deletes it
-		CheckDestroy: testAccCheckAWSDBInstanceSnapshot,
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSDBInstanceNoSnapshot,
 		Steps: []resource.TestStep{
 			{
 				Config: testAccSnapshotInstanceConfig(),
@@ -170,18 +168,20 @@ func TestAccAWSDBInstanceSnapshot(t *testing.T) {
 	})
 }
 
-func TestAccAWSDBInstanceNoSnapshot(t *testing.T) {
-	var nosnap rds.DBInstance
+func TestAccAWSDBInstanceSnapshot(t *testing.T) {
+	var snap rds.DBInstance
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:     func() { testAccPreCheck(t) },
-		Providers:    testAccProviders,
-		CheckDestroy: testAccCheckAWSDBInstanceNoSnapshot,
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		// testAccCheckAWSDBInstanceSnapshot verifies a database snapshot is
+		// created, and subequently deletes it
+		CheckDestroy: testAccCheckAWSDBInstanceSnapshot,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccNoSnapshotInstanceConfig(),
+				Config: testAccSnapshotInstanceConfigWithSnapshot(),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAWSDBInstanceExists("aws_db_instance.no_snapshot", &nosnap),
+					testAccCheckAWSDBInstanceExists("aws_db_instance.snapshot", &snap),
 				),
 			},
 		},
@@ -442,7 +442,7 @@ func testAccCheckAWSDBInstanceSnapshot(s *terraform.State) error {
 		}
 
 		log.Printf("[INFO] Trying to locate the DBInstance Final Snapshot")
-		snapshot_identifier := "foobarbaz-test-terraform-final-snapshot-1"
+		snapshot_identifier := "foobarbaz-test-terraform-final-snapshot-2"
 		_, snapErr := conn.DescribeDBSnapshots(
 			&rds.DescribeDBSnapshotsInput{
 				DBSnapshotIdentifier: aws.String(snapshot_identifier),
@@ -593,6 +593,7 @@ resource "aws_db_instance" "bar" {
 	# documented. Terraform will downcase this to match (as opposed to throw a 
 	# validation error).
 	maintenance_window = "Fri:09:00-Fri:09:30"
+	skip_final_snapshot = true
 
 	backup_retention_period = 0
 
@@ -640,6 +641,8 @@ resource "aws_db_instance" "bar" {
 	storage_encrypted = true
 	kms_key_id = "${aws_kms_key.foo.arn}"
 
+	skip_final_snapshot = true
+
 	parameter_group_name = "default.mysql5.6"
 }
 `
@@ -664,6 +667,7 @@ resource "aws_db_instance" "bar" {
 	username = "foo"
 
 	backup_retention_period = 0
+	skip_final_snapshot = true
 
 	parameter_group_name = "default.mysql5.6"
 	option_group_name = "${aws_db_option_group.bar.name}"
@@ -684,6 +688,7 @@ func testAccReplicaInstanceConfig(val int) string {
 		username = "foo"
 
 		backup_retention_period = 1
+		skip_final_snapshot = true
 
 		parameter_group_name = "default.mysql5.6"
 	}
@@ -698,6 +703,7 @@ func testAccReplicaInstanceConfig(val int) string {
 		instance_class = "${aws_db_instance.bar.instance_class}"
 		password = "${aws_db_instance.bar.password}"
 		username = "${aws_db_instance.bar.username}"
+		skip_final_snapshot = true
 		tags {
 			Name = "tf-replica-db"
 		}
@@ -711,7 +717,7 @@ provider "aws" {
   region = "us-east-1"
 }
 resource "aws_db_instance" "snapshot" {
-	identifier = "tf-snapshot-%d"
+	identifier = "tf-test-%d"
 
 	allocated_storage = 5
 	engine = "mysql"
@@ -727,22 +733,18 @@ resource "aws_db_instance" "snapshot" {
 
 	parameter_group_name = "default.mysql5.6"
 
-	skip_final_snapshot = false
-	copy_tags_to_snapshot = true
+	skip_final_snapshot = true
 	final_snapshot_identifier = "foobarbaz-test-terraform-final-snapshot-1"
-	tags {
-		Name = "tf-tags-db"
-	}
 }`, acctest.RandInt())
 }
 
-func testAccNoSnapshotInstanceConfig() string {
+func testAccSnapshotInstanceConfigWithSnapshot() string {
 	return fmt.Sprintf(`
 provider "aws" {
   region = "us-east-1"
 }
-resource "aws_db_instance" "no_snapshot" {
-	identifier = "tf-test-%s"
+resource "aws_db_instance" "snapshot" {
+	identifier = "tf-snapshot-%d"
 
 	allocated_storage = 5
 	engine = "mysql"
@@ -752,15 +754,18 @@ resource "aws_db_instance" "no_snapshot" {
 	password = "barbarbarbar"
 	publicly_accessible = true
 	username = "foo"
-    security_group_names = ["default"]
+    	security_group_names = ["default"]
 	backup_retention_period = 1
 
 	parameter_group_name = "default.mysql5.6"
 
-	skip_final_snapshot = true
+	copy_tags_to_snapshot = true
 	final_snapshot_identifier = "foobarbaz-test-terraform-final-snapshot-2"
+	tags {
+		Name = "tf-tags-db"
+	}
 }
-`, acctest.RandString(5))
+`, acctest.RandInt())
 }
 
 func testAccSnapshotInstanceConfig_enhancedMonitoring(rName string) string {
@@ -827,6 +832,7 @@ resource "aws_db_instance" "bar" {
   username             = "foo"
   password             = "barbarbar"
   parameter_group_name = "default.mysql5.6"
+  skip_final_snapshot = true
 
   apply_immediately = true
 
@@ -849,6 +855,7 @@ resource "aws_db_instance" "bar" {
   parameter_group_name = "default.mysql5.6"
   port = 3306
   allocated_storage = 10
+  skip_final_snapshot = true
 
   apply_immediately = true
 }`, rName)
@@ -867,6 +874,7 @@ resource "aws_db_instance" "bar" {
   parameter_group_name = "default.mysql5.6"
   port = 3305
   allocated_storage = 10
+  skip_final_snapshot = true
 
   apply_immediately = true
 }`, rName)
@@ -916,6 +924,7 @@ resource "aws_db_instance" "bar" {
   db_subnet_group_name = "${aws_db_subnet_group.foo.name}"
   port = 3305
   allocated_storage = 10
+  skip_final_snapshot = true
 
 	backup_retention_period = 0
   apply_immediately = true
@@ -996,6 +1005,7 @@ resource "aws_db_instance" "bar" {
   db_subnet_group_name = "${aws_db_subnet_group.bar.name}"
   port = 3305
   allocated_storage = 10
+  skip_final_snapshot = true
 
 	backup_retention_period = 0
 
@@ -1046,6 +1056,7 @@ resource "aws_db_instance" "mssql" {
   password                = "somecrazypassword"
   engine                  = "sqlserver-ex"
   backup_retention_period = 0
+  skip_final_snapshot = true
 
   #publicly_accessible = true
 
@@ -1113,6 +1124,7 @@ resource "aws_db_instance" "mssql" {
   password                = "somecrazypassword"
   engine                  = "sqlserver-ex"
   backup_retention_period = 0
+  skip_final_snapshot = true
 
   #publicly_accessible = true
 
@@ -1148,5 +1160,6 @@ resource "aws_db_instance" "bar" {
 	name = "baz"
 	password = "barbarbarbar"
 	username = "foo"
+	skip_final_snapshot = true
 }
 `, acctest.RandInt())

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

@@ -60,7 +60,7 @@ The following arguments are supported:
 * `final_snapshot_identifier` - (Optional) The name of your final DB snapshot
     when this DB instance 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 instance is deleted. If true is specified, no DBSnapshot is created. If false is specified, a DB snapshot is created before the DB instance 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 instance is deleted. If true is specified, no DBSnapshot is created. If false is specified, a DB snapshot is created before the DB instance is deleted, using the value from `final_snapshot_identifier`. Default is `false`.
 * `copy_tags_to_snapshot` – (Optional, boolean) On delete, copy all Instance `tags` to
 the final snapshot (if `final_snapshot_identifier` is specified). Default
 `false`