From 254b25a64faea06cef5642ce558497fc805fe37a Mon Sep 17 00:00:00 2001 From: Phil Frost Date: Wed, 28 Jan 2015 11:48:30 -0500 Subject: [PATCH] Fix aws_db_instance to not recreate each time Several of the arguments were optional, and if omitted, they are calculated. Mark them as such in the schema to avoid triggering an update. Go back to storing the password in the state file. Without doing so, there's no way for Terraform to know the password has changed. It should be hashed, but then interpolating the password yields a hash instead of the password. Make the `name` parameter optional. It's not required in any engine, and in some (MS SQL Server) it's not allowed at all. Drop the `skip_final_snapshot` argument. If `final_snapshot_identifier` isn't specified, then don't make a final snapshot. As things were, it was possible to create a resource with neither of these arguments specified which would later fail when it was to be deleted since the RDS API requires exactly one of the two. Resolves issue #689. --- .../providers/aws/resource_aws_db_instance.go | 24 +++++++++---------- .../aws/resource_aws_db_instance_test.go | 4 ---- .../providers/aws/r/db_instance.html.markdown | 12 ++++++---- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/builtin/providers/aws/resource_aws_db_instance.go b/builtin/providers/aws/resource_aws_db_instance.go index c06582aff..845efad59 100644 --- a/builtin/providers/aws/resource_aws_db_instance.go +++ b/builtin/providers/aws/resource_aws_db_instance.go @@ -21,7 +21,7 @@ func resourceAwsDbInstance() *schema.Resource { Schema: map[string]*schema.Schema{ "name": &schema.Schema{ Type: schema.TypeString, - Required: true, + Optional: true, ForceNew: true, }, @@ -70,18 +70,21 @@ func resourceAwsDbInstance() *schema.Resource { "availability_zone": &schema.Schema{ Type: schema.TypeString, Optional: true, + Computed: true, ForceNew: true, }, "backup_retention_period": &schema.Schema{ Type: schema.TypeInt, Optional: true, + Computed: true, ForceNew: true, }, "backup_window": &schema.Schema{ Type: schema.TypeString, Optional: true, + Computed: true, ForceNew: true, }, @@ -94,18 +97,21 @@ func resourceAwsDbInstance() *schema.Resource { "maintenance_window": &schema.Schema{ Type: schema.TypeString, Optional: true, + Computed: true, ForceNew: true, }, "multi_az": &schema.Schema{ Type: schema.TypeBool, Optional: true, + Computed: true, ForceNew: true, }, "port": &schema.Schema{ Type: schema.TypeInt, Optional: true, + Computed: true, ForceNew: true, }, @@ -133,12 +139,6 @@ func resourceAwsDbInstance() *schema.Resource { }, }, - "skip_final_snapshot": &schema.Schema{ - Type: schema.TypeBool, - Optional: true, - ForceNew: true, - }, - "final_snapshot_identifier": &schema.Schema{ Type: schema.TypeString, Optional: true, @@ -154,6 +154,7 @@ func resourceAwsDbInstance() *schema.Resource { "parameter_group_name": &schema.Schema{ Type: schema.TypeString, Optional: true, + Computed: true, ForceNew: true, }, @@ -189,10 +190,6 @@ func resourceAwsDbInstanceCreate(d *schema.ResourceData, meta interface{}) error EngineVersion: d.Get("engine_version").(string), } - // Special treatment for the password, as we don't want that - // saved into the state file - d.Set("password", "") - if attr, ok := d.GetOk("backup_retention_period"); ok { opts.BackupRetentionPeriod = attr.(int) opts.SetBackupRetentionPeriod = true @@ -344,10 +341,11 @@ func resourceAwsDbInstanceDelete(d *schema.ResourceData, meta interface{}) error opts := rds.DeleteDBInstance{DBInstanceIdentifier: d.Id()} - if d.Get("skip_final_snapshot").(bool) { + finalSnapshot := d.Get("final_snapshot_identifier").(string) + if finalSnapshot == "" { opts.SkipFinalSnapshot = true } else { - opts.FinalDBSnapshotIdentifier = d.Get("final_snapshot_identifier").(string) + opts.FinalDBSnapshotIdentifier = finalSnapshot } log.Printf("[DEBUG] DB Instance destroy configuration: %v", opts) diff --git a/builtin/providers/aws/resource_aws_db_instance_test.go b/builtin/providers/aws/resource_aws_db_instance_test.go index c2c1ae11c..8af1b036d 100644 --- a/builtin/providers/aws/resource_aws_db_instance_test.go +++ b/builtin/providers/aws/resource_aws_db_instance_test.go @@ -39,8 +39,6 @@ func TestAccAWSDBInstance(t *testing.T) { "aws_db_instance.bar", "password", ""), resource.TestCheckResourceAttr( "aws_db_instance.bar", "username", "foo"), - resource.TestCheckResourceAttr( - "aws_db_instance.bar", "skip_final_snapshot", "true"), resource.TestCheckResourceAttr( "aws_db_instance.bar", "security_group_names.3322503515", "secfoobarbaz-test-terraform"), resource.TestCheckResourceAttr( @@ -159,8 +157,6 @@ resource "aws_db_instance" "bar" { password = "barbarbarbar" username = "foo" - skip_final_snapshot = true - security_group_names = ["${aws_db_security_group.bar.name}"] parameter_group_name = "default.mysql5.6" } diff --git a/website/source/docs/providers/aws/r/db_instance.html.markdown b/website/source/docs/providers/aws/r/db_instance.html.markdown index 167c8823b..635d8137f 100644 --- a/website/source/docs/providers/aws/r/db_instance.html.markdown +++ b/website/source/docs/providers/aws/r/db_instance.html.markdown @@ -37,10 +37,13 @@ The following arguments are supported: * `engine_version` - (Required) The engine version to use. * `identifier` - (Required) The name of the RDS instance * `instance_class` - (Required) The instance type of the RDS instance. -* `final_snapshot_identifier` - (Optional) The name of your final DB snapshot. -* `name` - (Required) The DB name to create. -* `password` - (Required) Password for the master DB user. Note that this will be stored - in the state file. +* `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. +* `name` - (Optional) The DB name to create. If omitted, no database is created + initially. +* `password` - (Required) Password for the master DB user. Note that this may + show up in logs, and it will be stored in the state file. * `username` - (Required) Username for the master DB user. * `availability_zone` - (Optional) The AZ for the RDS instance. * `backup_retention_period` - (Optional) The days to retain backups for. @@ -51,7 +54,6 @@ The following arguments are supported: * `port` - (Optional) The port on which the DB accepts connections. * `publicly_accessible` - (Optional) Bool to control if instance is publicly accessible. * `vpc_security_group_ids` - (Optional) List of VPC security groups to associate. -* `skip_final_snapshot` - (Optional) Enables skipping the final snapshot on deletion. * `security_group_names` - (Optional) List of DB Security Groups to associate. * `db_subnet_group_name` - (Optional) Name of DB subnet group * `parameter_group_name` - (Optional) Name of the DB parameter group to associate.