From 09eb4862ea45c64c6f380246c149fb3d601513ca Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Mon, 6 Dec 2021 13:34:33 -0800 Subject: [PATCH 01/15] Disallows setting `name` when creating a replica (i.e. `replicate_source_db` is set) --- internal/service/rds/instance.go | 3 +++ website/docs/r/db_instance.html.markdown | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/service/rds/instance.go b/internal/service/rds/instance.go index 8b80f5cc30c..27cd89ccb42 100644 --- a/internal/service/rds/instance.go +++ b/internal/service/rds/instance.go @@ -291,6 +291,9 @@ func ResourceInstance() *schema.Resource { Optional: true, Computed: true, ForceNew: true, + ConflictsWith: []string{ + "replicate_source_db", + }, }, "nchar_character_set_name": { Type: schema.TypeString, diff --git a/website/docs/r/db_instance.html.markdown b/website/docs/r/db_instance.html.markdown index fe63d669c4f..747ff86f423 100644 --- a/website/docs/r/db_instance.html.markdown +++ b/website/docs/r/db_instance.html.markdown @@ -153,7 +153,7 @@ information on the [AWS Documentation](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_Monitoring.html) what IAM permissions are needed to allow Enhanced Monitoring for RDS Instances. * `multi_az` - (Optional) Specifies if the RDS instance is multi-AZ -* `name` - (Optional) The name of the database to create when the DB instance is created. If this parameter is not specified, no database is created in the DB instance. Note that this does not apply for Oracle or SQL Server engines. See the [AWS documentation](https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/create-db-instance.html) for more details on what applies for those engines. If you are providing an Oracle db name, it needs to be in all upper case. +* `name` - (Optional) The name of the database to create when the DB instance is created. If this parameter is not specified, no database is created in the DB instance. Note that this does not apply for Oracle or SQL Server engines. See the [AWS documentation](https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/create-db-instance.html) for more details on what applies for those engines. If you are providing an Oracle db name, it needs to be in all upper case. Cannot be specified for a replca. * `nchar_character_set_name` - (Optional, Forces new resource) The national character set is used in the NCHAR, NVARCHAR2, and NCLOB data types for Oracle instances. This can't be changed. See [Oracle Character Sets Supported in Amazon RDS](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Appendix.OracleCharacterSets.html). * `option_group_name` - (Optional) Name of the DB option group to associate. From d5bb0383b71c5265061fba168f2e0754d5dbcca9 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 7 Dec 2021 15:15:35 -0800 Subject: [PATCH 02/15] Disallows setting `username` when creating a replica (i.e. `replicate_source_db` is set). Makes `replica_mode` `Computed` --- internal/service/rds/instance.go | 4 ++++ website/docs/r/db_instance.html.markdown | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/service/rds/instance.go b/internal/service/rds/instance.go index 27cd89ccb42..13a9d0991f2 100644 --- a/internal/service/rds/instance.go +++ b/internal/service/rds/instance.go @@ -345,6 +345,7 @@ func ResourceInstance() *schema.Resource { "replica_mode": { Type: schema.TypeString, Optional: true, + Computed: true, ValidateFunc: validation.StringInSlice(rds.ReplicaMode_Values(), false), }, "replicas": { @@ -479,6 +480,9 @@ func ResourceInstance() *schema.Resource { Optional: true, Computed: true, ForceNew: true, + ConflictsWith: []string{ + "replicate_source_db", + }, }, "vpc_security_group_ids": { Type: schema.TypeSet, diff --git a/website/docs/r/db_instance.html.markdown b/website/docs/r/db_instance.html.markdown index 747ff86f423..79e1185c93f 100644 --- a/website/docs/r/db_instance.html.markdown +++ b/website/docs/r/db_instance.html.markdown @@ -206,7 +206,7 @@ creation. See [MSSQL User Guide](http://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/CHAP_SQLServer.html#SQLServer.Concepts.General.TimeZone) for more information. * `username` - (Required unless a `snapshot_identifier` or `replicate_source_db` -is provided) Username for the master DB user. +is provided) Username for the master DB user. Cannot be specified for a replca. * `vpc_security_group_ids` - (Optional) List of VPC security groups to associate. * `customer_owned_ip_enabled` - (Optional) Indicates whether to enable a customer-owned IP address (CoIP) for an RDS on Outposts DB instance. See [CoIP for RDS on Outposts](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/rds-on-outposts.html#rds-on-outposts.coip) for more information. From 82717f66617174f73178ec5c16764bbf4251826d Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 8 Dec 2021 11:39:02 -0800 Subject: [PATCH 03/15] Updates plan-time validation --- internal/service/rds/enum.go | 14 ++++++++ internal/service/rds/instance.go | 43 ++++++++++++------------ website/docs/r/db_instance.html.markdown | 8 ++--- 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/internal/service/rds/enum.go b/internal/service/rds/enum.go index 8e186e3b466..e08b834fe1e 100644 --- a/internal/service/rds/enum.go +++ b/internal/service/rds/enum.go @@ -6,6 +6,20 @@ const ( ClusterRoleStatusPending = "PENDING" ) +const ( + StorageTypeStandard = "standard" + StorageTypeGp2 = "gp2" + StorageTypeIo1 = "io1" +) + +func StorageType_Values() []string { + return []string{ + StorageTypeStandard, + StorageTypeGp2, + StorageTypeIo1, + } +} + // https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/accessing-monitoring.html#Overview.DBInstance.Status. const ( InstanceStatusAvailable = "available" diff --git a/internal/service/rds/instance.go b/internal/service/rds/instance.go index 13a9d0991f2..29d93d0ce22 100644 --- a/internal/service/rds/instance.go +++ b/internal/service/rds/instance.go @@ -181,11 +181,13 @@ func ResourceInstance() *schema.Resource { value := v.(string) return strings.ToLower(value) }, + ConflictsWith: []string{"replicate_source_db"}, }, "engine_version": { - Type: schema.TypeString, - Optional: true, - Computed: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ConflictsWith: []string{"replicate_source_db"}, }, "engine_version_actual": { Type: schema.TypeString, @@ -287,13 +289,11 @@ func ResourceInstance() *schema.Resource { Computed: true, }, "name": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, - ConflictsWith: []string{ - "replicate_source_db", - }, + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ConflictsWith: []string{"replicate_source_db"}, }, "nchar_character_set_name": { Type: schema.TypeString, @@ -463,9 +463,10 @@ func ResourceInstance() *schema.Resource { ForceNew: true, }, "storage_type": { - Type: schema.TypeString, - Optional: true, - Computed: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateFunc: validation.StringInSlice(StorageType_Values(), false), }, "tags": tftags.TagsSchema(), "tags_all": tftags.TagsSchemaComputed(), @@ -476,13 +477,11 @@ func ResourceInstance() *schema.Resource { ForceNew: true, }, "username": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, - ConflictsWith: []string{ - "replicate_source_db", - }, + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ConflictsWith: []string{"replicate_source_db"}, }, "vpc_security_group_ids": { Type: schema.TypeSet, @@ -544,10 +543,10 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { } if _, ok := d.GetOk("allocated_storage"); ok { - log.Printf("[INFO] allocated_storage was ignored for DB Instance (%s) because a replica inherits the primary's allocated_storage and this cannot be changed at creation.", d.Id()) // RDS doesn't allow modifying the storage of a replica within the first 6h of creation. // allocated_storage is inherited from the primary so only the same value or no value is correct; a different value would fail the creation. // A different value is possible, granted: the value is higher than the current, there has been 6h between + log.Printf("[INFO] allocated_storage was ignored for DB Instance (%s) because a replica inherits the primary's allocated_storage and this cannot be changed at creation.", d.Id()) } if attr, ok := d.GetOk("availability_zone"); ok { @@ -1493,8 +1492,8 @@ func resourceInstanceRead(d *schema.ResourceData, meta interface{}) error { sgn.Add(*v.DBSecurityGroupName) } d.Set("security_group_names", sgn) - // replica things + // replica things var replicas []string for _, v := range v.ReadReplicaDBInstanceIdentifiers { replicas = append(replicas, *v) diff --git a/website/docs/r/db_instance.html.markdown b/website/docs/r/db_instance.html.markdown index 79e1185c93f..2bfe0d823e4 100644 --- a/website/docs/r/db_instance.html.markdown +++ b/website/docs/r/db_instance.html.markdown @@ -110,7 +110,7 @@ for additional read replica contraints. * `domain_iam_role_name` - (Optional, but required if domain is provided) The name of the IAM role to be used when making API calls to the Directory Service. * `enabled_cloudwatch_logs_exports` - (Optional) Set of log types to enable for exporting to CloudWatch logs. If omitted, no logs will be exported. Valid values (depending on `engine`). MySQL and MariaDB: `audit`, `error`, `general`, `slowquery`. PostgreSQL: `postgresql`, `upgrade`. MSSQL: `agent` , `error`. Oracle: `alert`, `audit`, `listener`, `trace`. * `engine` - (Required unless a `snapshot_identifier` or `replicate_source_db` -is provided) The database engine to use. For supported values, see the Engine parameter in [API action CreateDBInstance](https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_CreateDBInstance.html). +is provided) The database engine to use. For supported values, see the Engine parameter in [API action CreateDBInstance](https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_CreateDBInstance.html). Cannot be specified for a replica. Note that for Amazon Aurora instances the engine must match the [DB cluster](/docs/providers/aws/r/rds_cluster.html)'s engine'. For information on the difference between the available Aurora MySQL engines see [Comparison between Aurora MySQL 1 and Aurora MySQL 2](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/AuroraMySQL.Updates.20180206.html) @@ -119,7 +119,7 @@ in the Amazon RDS User Guide. is enabled, you can provide a prefix of the version such as `5.7` (for `5.7.10`). The actual engine version used is returned in the attribute `engine_version_actual`, [defined below](#engine_version_actual). For supported values, see the EngineVersion parameter in [API action CreateDBInstance](https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_CreateDBInstance.html). -Note that for Amazon Aurora instances the engine version must match the [DB cluster](/docs/providers/aws/r/rds_cluster.html)'s engine version'. +Note that for Amazon Aurora instances the engine version must match the [DB cluster](/docs/providers/aws/r/rds_cluster.html)'s engine version'. Cannot be specified for a replica. * `final_snapshot_identifier` - (Optional) The name of your final DB snapshot when this DB instance is deleted. Must be provided if `skip_final_snapshot` is set to `false`. The value must begin with a letter, only contain alphanumeric characters and hyphens, and not end with a hyphen or contain two consecutive hyphens. Must not be provided when deleting a read replica. @@ -153,7 +153,7 @@ information on the [AWS Documentation](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_Monitoring.html) what IAM permissions are needed to allow Enhanced Monitoring for RDS Instances. * `multi_az` - (Optional) Specifies if the RDS instance is multi-AZ -* `name` - (Optional) The name of the database to create when the DB instance is created. If this parameter is not specified, no database is created in the DB instance. Note that this does not apply for Oracle or SQL Server engines. See the [AWS documentation](https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/create-db-instance.html) for more details on what applies for those engines. If you are providing an Oracle db name, it needs to be in all upper case. Cannot be specified for a replca. +* `name` - (Optional) The name of the database to create when the DB instance is created. If this parameter is not specified, no database is created in the DB instance. Note that this does not apply for Oracle or SQL Server engines. See the [AWS documentation](https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/create-db-instance.html) for more details on what applies for those engines. If you are providing an Oracle db name, it needs to be in all upper case. Cannot be specified for a replica. * `nchar_character_set_name` - (Optional, Forces new resource) The national character set is used in the NCHAR, NVARCHAR2, and NCLOB data types for Oracle instances. This can't be changed. See [Oracle Character Sets Supported in Amazon RDS](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Appendix.OracleCharacterSets.html). * `option_group_name` - (Optional) Name of the DB option group to associate. @@ -206,7 +206,7 @@ creation. See [MSSQL User Guide](http://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/CHAP_SQLServer.html#SQLServer.Concepts.General.TimeZone) for more information. * `username` - (Required unless a `snapshot_identifier` or `replicate_source_db` -is provided) Username for the master DB user. Cannot be specified for a replca. +is provided) Username for the master DB user. Cannot be specified for a replica. * `vpc_security_group_ids` - (Optional) List of VPC security groups to associate. * `customer_owned_ip_enabled` - (Optional) Indicates whether to enable a customer-owned IP address (CoIP) for an RDS on Outposts DB instance. See [CoIP for RDS on Outposts](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/rds-on-outposts.html#rds-on-outposts.coip) for more information. From fcf33b22481d1166f4908bfa1a5a16944eb4ec5d Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 8 Dec 2021 17:11:50 -0800 Subject: [PATCH 04/15] Only modify RDS Instance replica properties if needed --- internal/service/rds/instance.go | 153 +++++--- internal/service/rds/instance_test.go | 506 +++++++++++++++++++++++--- 2 files changed, 548 insertions(+), 111 deletions(-) diff --git a/internal/service/rds/instance.go b/internal/service/rds/instance.go index 29d93d0ce22..9d812830c0a 100644 --- a/internal/service/rds/instance.go +++ b/internal/service/rds/instance.go @@ -553,22 +553,6 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { opts.AvailabilityZone = aws.String(attr.(string)) } - if attr, ok := d.GetOk("allow_major_version_upgrade"); ok { - modifyDbInstanceInput.AllowMajorVersionUpgrade = aws.Bool(attr.(bool)) - // Having allowing_major_version_upgrade by itself should not trigger ModifyDBInstance - // InvalidParameterCombination: No modifications were requested - } - - if attr, ok := d.GetOk("backup_retention_period"); ok { - modifyDbInstanceInput.BackupRetentionPeriod = aws.Int64(int64(attr.(int))) - requiresModifyDbInstance = true - } - - if attr, ok := d.GetOk("backup_window"); ok { - modifyDbInstanceInput.PreferredBackupWindow = aws.String(attr.(string)) - requiresModifyDbInstance = true - } - if attr, ok := d.GetOk("db_subnet_group_name"); ok { opts.DBSubnetGroupName = aws.String(attr.(string)) } @@ -592,16 +576,6 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { } } - if attr, ok := d.GetOk("maintenance_window"); ok { - modifyDbInstanceInput.PreferredMaintenanceWindow = aws.String(attr.(string)) - requiresModifyDbInstance = true - } - - if attr, ok := d.GetOk("max_allocated_storage"); ok { - modifyDbInstanceInput.MaxAllocatedStorage = aws.Int64(int64(attr.(int))) - requiresModifyDbInstance = true - } - if attr, ok := d.GetOk("monitoring_interval"); ok { opts.MonitoringInterval = aws.Int64(int64(attr.(int))) } @@ -618,26 +592,10 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { opts.OptionGroupName = aws.String(attr.(string)) } - if attr, ok := d.GetOk("parameter_group_name"); ok { - modifyDbInstanceInput.DBParameterGroupName = aws.String(attr.(string)) - requiresModifyDbInstance = true - requiresRebootDbInstance = true - } - - if attr, ok := d.GetOk("password"); ok { - modifyDbInstanceInput.MasterUserPassword = aws.String(attr.(string)) - requiresModifyDbInstance = true - } - if attr, ok := d.GetOk("port"); ok { opts.Port = aws.Int64(int64(attr.(int))) } - if attr := d.Get("security_group_names").(*schema.Set); attr.Len() > 0 { - modifyDbInstanceInput.DBSecurityGroups = flex.ExpandStringSet(attr) - requiresModifyDbInstance = true - } - if attr, ok := d.GetOk("storage_type"); ok { opts.StorageType = aws.String(attr.(string)) } @@ -658,20 +616,91 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { opts.PerformanceInsightsRetentionPeriod = aws.Int64(int64(attr.(int))) } - if attr, ok := d.GetOk("ca_cert_identifier"); ok { - modifyDbInstanceInput.CACertificateIdentifier = aws.String(attr.(string)) - requiresModifyDbInstance = true - } - if attr, ok := d.GetOk("replica_mode"); ok { opts.ReplicaMode = aws.String(attr.(string)) requiresModifyDbInstance = true } - log.Printf("[DEBUG] DB Instance Replica create configuration: %#v", opts) - _, err := conn.CreateDBInstanceReadReplica(&opts) + output, err := conn.CreateDBInstanceReadReplica(&opts) if err != nil { - return fmt.Errorf("Error creating DB Instance: %s", err) + return fmt.Errorf("Error creating DB Instance: %w", err) + } + + if attr, ok := d.GetOk("allow_major_version_upgrade"); ok { + // Having allowing_major_version_upgrade by itself should not trigger ModifyDBInstance + // InvalidParameterCombination: No modifications were requested + modifyDbInstanceInput.AllowMajorVersionUpgrade = aws.Bool(attr.(bool)) + } + + if attr, ok := d.GetOk("backup_retention_period"); ok { + current := aws.Int64Value(output.DBInstance.BackupRetentionPeriod) + desired := int64(attr.(int)) + if current != desired { + modifyDbInstanceInput.BackupRetentionPeriod = aws.Int64(desired) + requiresModifyDbInstance = true + } + } + + if attr, ok := d.GetOk("backup_window"); ok { + current := aws.StringValue(output.DBInstance.PreferredBackupWindow) + desired := attr.(string) + if current != desired { + modifyDbInstanceInput.PreferredBackupWindow = aws.String(desired) + requiresModifyDbInstance = true + } + } + + if attr, ok := d.GetOk("ca_cert_identifier"); ok { + current := aws.StringValue(output.DBInstance.CACertificateIdentifier) + desired := attr.(string) + if current != desired { + modifyDbInstanceInput.CACertificateIdentifier = aws.String(desired) + requiresModifyDbInstance = true + } + } + + if attr, ok := d.GetOk("maintenance_window"); ok { + current := aws.StringValue(output.DBInstance.PreferredMaintenanceWindow) + desired := attr.(string) + if current != desired { + modifyDbInstanceInput.PreferredMaintenanceWindow = aws.String(desired) + requiresModifyDbInstance = true + } + } + + if attr, ok := d.GetOk("max_allocated_storage"); ok { + current := aws.Int64Value(output.DBInstance.MaxAllocatedStorage) + desired := int64(attr.(int)) + if current != desired { + modifyDbInstanceInput.MaxAllocatedStorage = aws.Int64(desired) + requiresModifyDbInstance = true + } + } + + if attr, ok := d.GetOk("parameter_group_name"); ok { + if len(output.DBInstance.DBParameterGroups) > 0 { + current := aws.StringValue(output.DBInstance.DBParameterGroups[0].DBParameterGroupName) + desired := attr.(string) + if current != desired { + modifyDbInstanceInput.DBParameterGroupName = aws.String(desired) + requiresModifyDbInstance = true + requiresRebootDbInstance = true + } + } + } + + if attr, ok := d.GetOk("password"); ok { + modifyDbInstanceInput.MasterUserPassword = aws.String(attr.(string)) + requiresModifyDbInstance = true + } + + if attr := d.Get("security_group_names").(*schema.Set); attr.Len() > 0 { + current := flattenDBSecurityGroups(output.DBInstance.DBSecurityGroups) + desired := attr + if !desired.Equal(current) { + modifyDbInstanceInput.DBSecurityGroups = flex.ExpandStringSet(attr) + requiresModifyDbInstance = true + } } } else if v, ok := d.GetOk("s3_import"); ok { @@ -1172,6 +1201,7 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { attr := d.Get("backup_retention_period") opts.BackupRetentionPeriod = aws.Int64(int64(attr.(int))) + if attr, ok := d.GetOk("multi_az"); ok { opts.MultiAZ = aws.Bool(attr.(bool)) @@ -1484,14 +1514,7 @@ func resourceInstanceRead(d *schema.ResourceData, meta interface{}) error { } d.Set("vpc_security_group_ids", ids) - // Create an empty schema.Set to hold all security group names - sgn := &schema.Set{ - F: schema.HashString, - } - for _, v := range v.DBSecurityGroups { - sgn.Add(*v.DBSecurityGroupName) - } - d.Set("security_group_names", sgn) + d.Set("security_group_names", flattenDBSecurityGroups(v.DBSecurityGroups)) // replica things var replicas []string @@ -1499,7 +1522,7 @@ func resourceInstanceRead(d *schema.ResourceData, meta interface{}) error { replicas = append(replicas, *v) } if err := d.Set("replicas", replicas); err != nil { - return fmt.Errorf("Error setting replicas attribute: %#v, error: %#v", replicas, err) + return fmt.Errorf("Error setting replicas attribute: %#v, error: %w", replicas, err) } d.Set("replica_mode", v.ReplicaMode) @@ -1801,7 +1824,7 @@ func resourceInstanceUpdate(d *schema.ResourceData, meta interface{}) error { } _, err := conn.PromoteReadReplica(&opts) if err != nil { - return fmt.Errorf("Error promoting database: %#v", err) + return fmt.Errorf("Error promoting database: %w", err) } d.Set("replicate_source_db", "") } else { @@ -1952,3 +1975,13 @@ func dbSetResourceDataEngineVersionFromInstance(d *schema.ResourceData, c *rds.D newVersion := aws.StringValue(c.EngineVersion) compareActualEngineVersion(d, oldVersion, newVersion) } + +func flattenDBSecurityGroups(groups []*rds.DBSecurityGroupMembership) *schema.Set { + result := &schema.Set{ + F: schema.HashString, + } + for _, v := range groups { + result.Add(aws.StringValue(v.DBSecurityGroupName)) + } + return result +} diff --git a/internal/service/rds/instance_test.go b/internal/service/rds/instance_test.go index 6fce6f419c2..6e1ac483639 100644 --- a/internal/service/rds/instance_test.go +++ b/internal/service/rds/instance_test.go @@ -32,7 +32,7 @@ func TestAccRDSInstance_basic(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccInstanceBasicConfig(), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInstanceExists(resourceName, &dbInstance1), testAccCheckInstanceAttributes(&dbInstance1), resource.TestCheckResourceAttr(resourceName, "allocated_storage", "10"), @@ -64,6 +64,7 @@ func TestAccRDSInstance_basic(t *testing.T) { resource.TestCheckResourceAttrSet(resourceName, "resource_id"), resource.TestCheckResourceAttr(resourceName, "status", "available"), resource.TestCheckResourceAttr(resourceName, "storage_encrypted", "false"), + resource.TestCheckResourceAttr(resourceName, "storage_type", "gp2"), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), resource.TestCheckResourceAttr(resourceName, "username", "foo"), ), @@ -599,7 +600,7 @@ func TestAccRDSInstance_password(t *testing.T) { }) } -func TestAccRDSInstance_replicateSourceDB(t *testing.T) { +func TestAccRDSInstance_replicateSourceDB_basic(t *testing.T) { var dbInstance, sourceDbInstance rds.DBInstance rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -613,8 +614,41 @@ func TestAccRDSInstance_replicateSourceDB(t *testing.T) { CheckDestroy: testAccCheckInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccInstanceConfig_ReplicateSourceDB(rName), - Check: resource.ComposeTestCheckFunc( + Config: testAccInstanceConfig_ReplicateSourceDB_basic(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExists(sourceResourceName, &sourceDbInstance), + testAccCheckInstanceExists(resourceName, &dbInstance), + testAccCheckInstanceReplicaAttributes(&sourceDbInstance, &dbInstance), + resource.TestCheckResourceAttrPair(resourceName, "name", sourceResourceName, "name"), + resource.TestCheckResourceAttrPair(resourceName, "username", sourceResourceName, "username"), + ), + }, + }, + }) +} + +func TestAccRDSInstance_replicateSourceDB_addLater(t *testing.T) { + var dbInstance, sourceDbInstance rds.DBInstance + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + sourceResourceName := "aws_db_instance.source" + resourceName := "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_ReplicateSourceDB_addLaterSetup(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExists(sourceResourceName, &sourceDbInstance), + ), + }, + { + Config: testAccInstanceConfig_ReplicateSourceDB_addLater(rName), + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInstanceExists(sourceResourceName, &sourceDbInstance), testAccCheckInstanceExists(resourceName, &dbInstance), testAccCheckInstanceReplicaAttributes(&sourceDbInstance, &dbInstance), @@ -1102,7 +1136,7 @@ func TestAccRDSInstance_ReplicateSourceDB_multiAZ(t *testing.T) { }) } -func TestAccRDSInstance_ReplicateSourceDB_parameterGroupName(t *testing.T) { +func TestAccRDSInstance_ReplicateSourceDB_parameterGroupName_sameSetOnBoth(t *testing.T) { var dbInstance, sourceDbInstance rds.DBInstance rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -1116,8 +1150,94 @@ func TestAccRDSInstance_ReplicateSourceDB_parameterGroupName(t *testing.T) { CheckDestroy: testAccCheckInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccInstanceConfig_ReplicateSourceDB_ParameterGroupName(rName), - Check: resource.ComposeTestCheckFunc( + Config: testAccInstanceConfig_ReplicateSourceDB_ParameterGroupName_sameSetOnBoth(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExists(sourceResourceName, &sourceDbInstance), + testAccCheckInstanceExists(resourceName, &dbInstance), + testAccCheckInstanceReplicaAttributes(&sourceDbInstance, &dbInstance), + resource.TestCheckResourceAttr(resourceName, "parameter_group_name", rName), + resource.TestCheckResourceAttrPair(resourceName, "parameter_group_name", sourceResourceName, "parameter_group_name"), + testAccCheckInstanceParameterApplyStatusInSync(&dbInstance), + testAccCheckInstanceParameterApplyStatusInSync(&sourceDbInstance), + ), + }, + }, + }) +} + +func TestAccRDSInstance_ReplicateSourceDB_parameterGroupName_differentSetOnBoth(t *testing.T) { + var dbInstance, sourceDbInstance rds.DBInstance + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + sourceResourceName := "aws_db_instance.source" + resourceName := "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_ReplicateSourceDB_ParameterGroupName_differentSetOnBoth(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExists(sourceResourceName, &sourceDbInstance), + resource.TestCheckResourceAttr(sourceResourceName, "parameter_group_name", fmt.Sprintf("%s-source", rName)), + testAccCheckInstanceExists(resourceName, &dbInstance), + testAccCheckInstanceReplicaAttributes(&sourceDbInstance, &dbInstance), + resource.TestCheckResourceAttr(resourceName, "parameter_group_name", rName), + testAccCheckInstanceParameterApplyStatusInSync(&dbInstance), + testAccCheckInstanceParameterApplyStatusInSync(&sourceDbInstance), + ), + }, + }, + }) +} + +func TestAccRDSInstance_ReplicateSourceDB_parameterGroupName_replicaCopiesValue(t *testing.T) { + var dbInstance, sourceDbInstance rds.DBInstance + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + sourceResourceName := "aws_db_instance.source" + resourceName := "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_ReplicateSourceDB_ParameterGroupName_replicaCopiesValue(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExists(sourceResourceName, &sourceDbInstance), + testAccCheckInstanceExists(resourceName, &dbInstance), + testAccCheckInstanceReplicaAttributes(&sourceDbInstance, &dbInstance), + resource.TestCheckResourceAttr(resourceName, "parameter_group_name", rName), + resource.TestCheckResourceAttrPair(resourceName, "parameter_group_name", sourceResourceName, "parameter_group_name"), + testAccCheckInstanceParameterApplyStatusInSync(&dbInstance), + ), + }, + }, + }) +} + +func TestAccRDSInstance_ReplicateSourceDB_parameterGroupName_setOnReplica(t *testing.T) { + var dbInstance, sourceDbInstance rds.DBInstance + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + sourceResourceName := "aws_db_instance.source" + resourceName := "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_ReplicateSourceDB_ParameterGroupName_setOnReplica(rName), + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInstanceExists(sourceResourceName, &sourceDbInstance), testAccCheckInstanceExists(resourceName, &dbInstance), testAccCheckInstanceReplicaAttributes(&sourceDbInstance, &dbInstance), @@ -1187,7 +1307,7 @@ func TestAccRDSInstance_ReplicateSourceDB_caCertificateIdentifier(t *testing.T) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) sourceResourceName := "aws_db_instance.source" resourceName := "aws_db_instance.test" - dataSourceName := "data.aws_rds_certificate.latest" + certifiateDataSourceName := "data.aws_rds_certificate.latest" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -1201,8 +1321,8 @@ func TestAccRDSInstance_ReplicateSourceDB_caCertificateIdentifier(t *testing.T) testAccCheckInstanceExists(sourceResourceName, &sourceDbInstance), testAccCheckInstanceExists(resourceName, &dbInstance), testAccCheckInstanceReplicaAttributes(&sourceDbInstance, &dbInstance), - resource.TestCheckResourceAttrPair(sourceResourceName, "ca_cert_identifier", dataSourceName, "id"), - resource.TestCheckResourceAttrPair(resourceName, "ca_cert_identifier", dataSourceName, "id"), + resource.TestCheckResourceAttrPair(sourceResourceName, "ca_cert_identifier", certifiateDataSourceName, "id"), + resource.TestCheckResourceAttrPair(resourceName, "ca_cert_identifier", certifiateDataSourceName, "id"), ), }, }, @@ -1235,6 +1355,49 @@ func TestAccRDSInstance_ReplicateSourceDB_replicaMode(t *testing.T) { }) } +// When an RDS Instance is added in a separate apply from the creation of the source instance, and the +// parameter group is changed on the replica, it can sometimes lead to the API trying to reboot the instance +// whenanother "management operation" is in progress: +// InvalidDBInstanceState: Instance cannot currently reboot due to an in-progress management operation +// https://github.com/hashicorp/terraform-provider-aws/issues/11905 +func TestAccRDSInstance_ReplicateSourceDB_parameterGroupTwoStep(t *testing.T) { + var dbInstance, sourceDbInstance rds.DBInstance + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_db_instance.test" + sourceResourceName := "aws_db_instance.source" + parameterGroupResourceName := "aws_db_parameter_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_ReplicateSourceDB_ParameterGroupTwoStep_Setup(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExists(sourceResourceName, &sourceDbInstance), + resource.TestCheckResourceAttr(sourceResourceName, "parameter_group_name", "default.oracle-ee-19"), + testAccCheckInstanceParameterApplyStatusInSync(&sourceDbInstance), + ), + }, + { + Config: testAccInstanceConfig_ReplicateSourceDB_ParameterGroupTwoStep(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExists(sourceResourceName, &sourceDbInstance), + resource.TestCheckResourceAttr(sourceResourceName, "parameter_group_name", "default.oracle-ee-19"), + testAccCheckInstanceExists(resourceName, &dbInstance), + resource.TestCheckResourceAttr(resourceName, "replica_mode", "open-read-only"), + resource.TestCheckResourceAttrPair(resourceName, "parameter_group_name", parameterGroupResourceName, "id"), + testAccCheckInstanceParameterApplyStatusInSync(&dbInstance), + testAccCheckInstanceParameterApplyStatusInSync(&sourceDbInstance), + ), + }, + }, + }) +} + func TestAccRDSInstance_s3Import(t *testing.T) { var snap rds.DBInstance bucket := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -3569,9 +3732,9 @@ func TestAccRDSInstance_license(t *testing.T) { func testAccInstanceConfig_orderableClass(engine, version, license string) string { return fmt.Sprintf(` data "aws_rds_orderable_db_instance" "test" { - engine = %q - engine_version = %q - license_model = %q + engine = %[1]q + engine_version = %[2]q + license_model = %[3]q storage_type = "standard" preferred_instance_classes = ["db.t3.micro", "db.t2.micro", "db.t2.medium"] @@ -3592,7 +3755,8 @@ func testAccInstanceConfig_orderableClassSQLServerEx() string { } func testAccInstanceBasicConfig() string { - return acctest.ConfigCompose(testAccInstanceConfig_orderableClassMySQL(), ` + return acctest.ConfigCompose( + testAccInstanceConfig_orderableClassMySQL(), ` resource "aws_db_instance" "bar" { allocated_storage = 10 backup_retention_period = 0 @@ -5561,8 +5725,51 @@ resource "aws_db_instance" "test" { `, rName, password)) } -func testAccInstanceConfig_ReplicateSourceDB(rName string) string { - return acctest.ConfigCompose(testAccInstanceConfig_orderableClassMySQL(), fmt.Sprintf(` +func testAccInstanceConfig_ReplicateSourceDB_basic(rName string) string { + return acctest.ConfigCompose( + testAccInstanceConfig_orderableClassMySQL(), + fmt.Sprintf(` +resource "aws_db_instance" "source" { + allocated_storage = 5 + backup_retention_period = 1 + engine = data.aws_rds_orderable_db_instance.test.engine + identifier = "%[1]s-source" + instance_class = data.aws_rds_orderable_db_instance.test.instance_class + password = "avoid-plaintext-passwords" + username = "tfacctest" + skip_final_snapshot = true +} + +resource "aws_db_instance" "test" { + identifier = %[1]q + instance_class = aws_db_instance.source.instance_class + replicate_source_db = aws_db_instance.source.id + skip_final_snapshot = true +} +`, rName)) +} + +func testAccInstanceConfig_ReplicateSourceDB_addLaterSetup(rName string) string { + return acctest.ConfigCompose( + testAccInstanceConfig_orderableClassMySQL(), + fmt.Sprintf(` +resource "aws_db_instance" "source" { + allocated_storage = 5 + backup_retention_period = 1 + engine = data.aws_rds_orderable_db_instance.test.engine + identifier = "%[1]s-source" + instance_class = data.aws_rds_orderable_db_instance.test.instance_class + password = "avoid-plaintext-passwords" + username = "tfacctest" + skip_final_snapshot = true +} +`, rName)) +} + +func testAccInstanceConfig_ReplicateSourceDB_addLater(rName string) string { + return acctest.ConfigCompose( + testAccInstanceConfig_orderableClassMySQL(), + fmt.Sprintf(` resource "aws_db_instance" "source" { allocated_storage = 5 backup_retention_period = 1 @@ -6269,34 +6476,10 @@ resource "aws_db_instance" "test" { } func testAccInstanceConfig_ReplicateSourceDB_Monitoring(rName string, monitoringInterval int) string { - return acctest.ConfigCompose(testAccInstanceConfig_orderableClassMySQL(), fmt.Sprintf(` -data "aws_partition" "current" {} - -resource "aws_iam_role" "test" { - name = %[1]q - - assume_role_policy = < Date: Wed, 8 Dec 2021 17:55:07 -0800 Subject: [PATCH 05/15] Updates documentation to use constant --- docs/contributing/contribution-checklists.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing/contribution-checklists.md b/docs/contributing/contribution-checklists.md index 37d1bb35bb2..e2f3a17fb04 100644 --- a/docs/contributing/contribution-checklists.md +++ b/docs/contributing/contribution-checklists.md @@ -148,7 +148,7 @@ func TestAccServiceThing_nameGenerated(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckThingExists(resourceName, &thing), create.TestCheckResourceAttrNameGenerated(resourceName, "name"), - resource.TestCheckResourceAttr(resourceName, "name_prefix", "terraform-"), + resource.TestCheckResourceAttr(resourceName, "name_prefix", resource.UniqueIdPrefix), ), }, // If the resource supports import: From f620e5557e5f457d2f9192ea06ad8eb3cf7e0f16 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 8 Dec 2021 17:56:04 -0800 Subject: [PATCH 06/15] Updates standard RDS Instance naming tests to match documentation --- internal/service/rds/instance.go | 2 + internal/service/rds/instance_test.go | 77 +++++++++++++++++++-------- 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/internal/service/rds/instance.go b/internal/service/rds/instance.go index 9d812830c0a..5d7e1b77636 100644 --- a/internal/service/rds/instance.go +++ b/internal/service/rds/instance.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/create" "github.com/hashicorp/terraform-provider-aws/internal/flex" tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" @@ -1417,6 +1418,7 @@ func resourceInstanceRead(d *schema.ResourceData, meta interface{}) error { d.Set("name", v.DBName) d.Set("identifier", v.DBInstanceIdentifier) + d.Set("identifier_prefix", create.NamePrefixFromName(aws.StringValue(v.DBInstanceIdentifier))) d.Set("resource_id", v.DbiResourceId) d.Set("username", v.MasterUsername) d.Set("deletion_protection", v.DeletionProtection) diff --git a/internal/service/rds/instance_test.go b/internal/service/rds/instance_test.go index 6e1ac483639..99b3b1e41b3 100644 --- a/internal/service/rds/instance_test.go +++ b/internal/service/rds/instance_test.go @@ -16,13 +16,16 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/create" tfrds "github.com/hashicorp/terraform-provider-aws/internal/service/rds" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccRDSInstance_basic(t *testing.T) { var dbInstance1 rds.DBInstance - resourceName := "aws_db_instance.bar" + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_db_instance.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -31,10 +34,12 @@ func TestAccRDSInstance_basic(t *testing.T) { CheckDestroy: testAccCheckInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccInstanceBasicConfig(), + Config: testAccInstanceBasicConfig(rName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInstanceExists(resourceName, &dbInstance1), testAccCheckInstanceAttributes(&dbInstance1), + resource.TestCheckResourceAttr(resourceName, "identifier", rName), + resource.TestCheckResourceAttr(resourceName, "identifier_prefix", ""), resource.TestCheckResourceAttr(resourceName, "allocated_storage", "10"), resource.TestCheckNoResourceAttr(resourceName, "allow_major_version_upgrade"), resource.TestCheckResourceAttr(resourceName, "auto_minor_version_upgrade", "true"), @@ -66,7 +71,7 @@ func TestAccRDSInstance_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "storage_encrypted", "false"), resource.TestCheckResourceAttr(resourceName, "storage_type", "gp2"), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), - resource.TestCheckResourceAttr(resourceName, "username", "foo"), + resource.TestCheckResourceAttr(resourceName, "username", "test"), ), }, { @@ -121,6 +126,9 @@ func TestAccRDSInstance_onlyMajorVersion(t *testing.T) { func TestAccRDSInstance_namePrefix(t *testing.T) { var v rds.DBInstance + const identifierPrefix = "tf-acc-test-prefix-" + const resourceName = "aws_db_instance.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), @@ -128,21 +136,30 @@ func TestAccRDSInstance_namePrefix(t *testing.T) { CheckDestroy: testAccCheckInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccInstanceConfig_namePrefix(), + Config: testAccInstanceConfig_namePrefix(identifierPrefix), Check: resource.ComposeTestCheckFunc( - testAccCheckInstanceExists("aws_db_instance.test", &v), - testAccCheckInstanceAttributes(&v), - resource.TestMatchResourceAttr( - "aws_db_instance.test", "identifier", regexp.MustCompile("^tf-test-")), + testAccCheckInstanceExists(resourceName, &v), + create.TestCheckResourceAttrNameFromPrefix(resourceName, "identifier", identifierPrefix), + resource.TestCheckResourceAttr(resourceName, "identifier_prefix", identifierPrefix), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "password", + }, + }, }, }) } -func TestAccRDSInstance_generatedName(t *testing.T) { +func TestAccRDSInstance_nameGenerated(t *testing.T) { var v rds.DBInstance + const resourceName = "aws_db_instance.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), @@ -150,12 +167,21 @@ func TestAccRDSInstance_generatedName(t *testing.T) { CheckDestroy: testAccCheckInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccInstanceConfig_generatedName(), + Config: testAccInstanceConfig_nameGenerated(), Check: resource.ComposeTestCheckFunc( - testAccCheckInstanceExists("aws_db_instance.test", &v), - testAccCheckInstanceAttributes(&v), + testAccCheckInstanceExists(resourceName, &v), + create.TestCheckResourceAttrNameGenerated(resourceName, "identifier"), + resource.TestCheckResourceAttr(resourceName, "identifier_prefix", resource.UniqueIdPrefix), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "password", + }, + }, }, }) } @@ -3754,10 +3780,12 @@ func testAccInstanceConfig_orderableClassSQLServerEx() string { return testAccInstanceConfig_orderableClass("sqlserver-ex", "14.00.1000.169.v1", "license-included") } -func testAccInstanceBasicConfig() string { +func testAccInstanceBasicConfig(rName string) string { return acctest.ConfigCompose( - testAccInstanceConfig_orderableClassMySQL(), ` -resource "aws_db_instance" "bar" { + testAccInstanceConfig_orderableClassMySQL(), + fmt.Sprintf(` +resource "aws_db_instance" "test" { + identifier = %[1]q allocated_storage = 10 backup_retention_period = 0 engine = data.aws_rds_orderable_db_instance.test.engine @@ -3767,14 +3795,14 @@ resource "aws_db_instance" "bar" { parameter_group_name = "default.mysql5.6" password = "barbarbarbar" skip_final_snapshot = true - username = "foo" + username = "test" # Maintenance Window is stored in lower case in the API, though not strictly # documented. Terraform will downcase this to match (as opposed to throw a # validation error). maintenance_window = "Fri:09:00-Fri:09:30" } -`) +`, rName)) } func testAccInstanceConfig_MajorVersionOnly(engine, engineVersion string) string { @@ -3803,23 +3831,26 @@ resource "aws_db_instance" "test" { `, engine, engineVersion)) } -func testAccInstanceConfig_namePrefix() string { - return acctest.ConfigCompose(testAccInstanceConfig_orderableClassMySQL(), ` +func testAccInstanceConfig_namePrefix(identifierPrefix string) string { + return acctest.ConfigCompose( + testAccInstanceConfig_orderableClassMySQL(), + fmt.Sprintf(` resource "aws_db_instance" "test" { + identifier_prefix = %[1]q allocated_storage = 10 engine = data.aws_rds_orderable_db_instance.test.engine - identifier_prefix = "tf-test-" instance_class = data.aws_rds_orderable_db_instance.test.instance_class password = "password" publicly_accessible = true skip_final_snapshot = true username = "root" } -`) +`, identifierPrefix)) } -func testAccInstanceConfig_generatedName() string { - return acctest.ConfigCompose(testAccInstanceConfig_orderableClassMySQL(), ` +func testAccInstanceConfig_nameGenerated() string { + return acctest.ConfigCompose( + testAccInstanceConfig_orderableClassMySQL(), ` resource "aws_db_instance" "test" { allocated_storage = 10 engine = data.aws_rds_orderable_db_instance.test.engine From 6bd9f4ceca6018b58650b804c353f80395d8744c Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 8 Dec 2021 23:22:42 -0800 Subject: [PATCH 07/15] Adds standard naming tests for RDS Instance replicas --- internal/service/rds/instance_test.go | 121 ++++++++++++++++++++++++-- 1 file changed, 115 insertions(+), 6 deletions(-) diff --git a/internal/service/rds/instance_test.go b/internal/service/rds/instance_test.go index 99b3b1e41b3..32e67c1a662 100644 --- a/internal/service/rds/instance_test.go +++ b/internal/service/rds/instance_test.go @@ -644,11 +644,86 @@ func TestAccRDSInstance_replicateSourceDB_basic(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInstanceExists(sourceResourceName, &sourceDbInstance), testAccCheckInstanceExists(resourceName, &dbInstance), + resource.TestCheckResourceAttr(resourceName, "identifier", rName), + resource.TestCheckResourceAttr(resourceName, "identifier_prefix", ""), testAccCheckInstanceReplicaAttributes(&sourceDbInstance, &dbInstance), resource.TestCheckResourceAttrPair(resourceName, "name", sourceResourceName, "name"), resource.TestCheckResourceAttrPair(resourceName, "username", sourceResourceName, "username"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "password", + }, + }, + }, + }) +} + +func TestAccRDSInstance_replicateSourceDB_namePrefix(t *testing.T) { + var v rds.DBInstance + + sourceName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + const identifierPrefix = "tf-acc-test-prefix-" + const resourceName = "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_ReplicateSourceDB_namePrefix(identifierPrefix, sourceName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + create.TestCheckResourceAttrNameFromPrefix(resourceName, "identifier", identifierPrefix), + resource.TestCheckResourceAttr(resourceName, "identifier_prefix", identifierPrefix), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "password", + }, + }, + }, + }) +} + +func TestAccRDSInstance_replicateSourceDB_nameGenerated(t *testing.T) { + var v rds.DBInstance + + sourceName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + const resourceName = "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_ReplicateSourceDB_nameGenerated(sourceName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + create.TestCheckResourceAttrNameGenerated(resourceName, "identifier"), + resource.TestCheckResourceAttr(resourceName, "identifier_prefix", resource.UniqueIdPrefix), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "password", + }, + }, }, }) } @@ -667,7 +742,7 @@ func TestAccRDSInstance_replicateSourceDB_addLater(t *testing.T) { CheckDestroy: testAccCheckInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccInstanceConfig_ReplicateSourceDB_addLaterSetup(rName), + Config: testAccInstanceConfig_ReplicateSourceDB_addLaterSetup(), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInstanceExists(sourceResourceName, &sourceDbInstance), ), @@ -5780,7 +5855,7 @@ resource "aws_db_instance" "test" { `, rName)) } -func testAccInstanceConfig_ReplicateSourceDB_addLaterSetup(rName string) string { +func testAccInstanceConfig_ReplicateSourceDB_namePrefix(identifierPrefix, sourceName string) string { return acctest.ConfigCompose( testAccInstanceConfig_orderableClassMySQL(), fmt.Sprintf(` @@ -5788,16 +5863,23 @@ resource "aws_db_instance" "source" { allocated_storage = 5 backup_retention_period = 1 engine = data.aws_rds_orderable_db_instance.test.engine - identifier = "%[1]s-source" + identifier = %[1]q instance_class = data.aws_rds_orderable_db_instance.test.instance_class password = "avoid-plaintext-passwords" username = "tfacctest" skip_final_snapshot = true } -`, rName)) + +resource "aws_db_instance" "test" { + identifier_prefix = %[2]q + instance_class = aws_db_instance.source.instance_class + replicate_source_db = aws_db_instance.source.id + skip_final_snapshot = true +} +`, sourceName, identifierPrefix)) } -func testAccInstanceConfig_ReplicateSourceDB_addLater(rName string) string { +func testAccInstanceConfig_ReplicateSourceDB_nameGenerated(sourceName string) string { return acctest.ConfigCompose( testAccInstanceConfig_orderableClassMySQL(), fmt.Sprintf(` @@ -5805,13 +5887,40 @@ resource "aws_db_instance" "source" { allocated_storage = 5 backup_retention_period = 1 engine = data.aws_rds_orderable_db_instance.test.engine - identifier = "%[1]s-source" + identifier = %[1]q + instance_class = data.aws_rds_orderable_db_instance.test.instance_class + password = "avoid-plaintext-passwords" + username = "tfacctest" + skip_final_snapshot = true +} + +resource "aws_db_instance" "test" { + instance_class = aws_db_instance.source.instance_class + replicate_source_db = aws_db_instance.source.id + skip_final_snapshot = true +} +`, sourceName)) +} + +func testAccInstanceConfig_ReplicateSourceDB_addLaterSetup() string { + return acctest.ConfigCompose( + testAccInstanceConfig_orderableClassMySQL(), ` +resource "aws_db_instance" "source" { + allocated_storage = 5 + backup_retention_period = 1 + engine = data.aws_rds_orderable_db_instance.test.engine instance_class = data.aws_rds_orderable_db_instance.test.instance_class password = "avoid-plaintext-passwords" username = "tfacctest" skip_final_snapshot = true } +`) +} +func testAccInstanceConfig_ReplicateSourceDB_addLater(rName string) string { + return acctest.ConfigCompose( + testAccInstanceConfig_ReplicateSourceDB_addLaterSetup(), + fmt.Sprintf(` resource "aws_db_instance" "test" { identifier = %[1]q instance_class = aws_db_instance.source.instance_class From 0eaeb107bf507bd31640fedafe947257d562d6eb Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 9 Dec 2021 17:15:06 -0800 Subject: [PATCH 08/15] Adds standard naming tests for RDS Instance snapshot restores --- internal/service/rds/instance_test.go | 179 +++++++++++++++++++++++--- 1 file changed, 162 insertions(+), 17 deletions(-) diff --git a/internal/service/rds/instance_test.go b/internal/service/rds/instance_test.go index 32e67c1a662..ee1d4b379e4 100644 --- a/internal/service/rds/instance_test.go +++ b/internal/service/rds/instance_test.go @@ -1521,7 +1521,7 @@ func TestAccRDSInstance_s3Import(t *testing.T) { }) } -func TestAccRDSInstance_snapshotIdentifier(t *testing.T) { +func TestAccRDSInstance_SnapshotIdentifier_basic(t *testing.T) { var dbInstance, sourceDbInstance rds.DBInstance var dbSnapshot rds.DBSnapshot @@ -1542,16 +1542,96 @@ func TestAccRDSInstance_snapshotIdentifier(t *testing.T) { testAccCheckInstanceExists(sourceDbResourceName, &sourceDbInstance), testAccCheckDbSnapshotExists(snapshotResourceName, &dbSnapshot), testAccCheckInstanceExists(resourceName, &dbInstance), + resource.TestCheckResourceAttr(resourceName, "identifier", rName), + resource.TestCheckResourceAttr(resourceName, "identifier_prefix", ""), + resource.TestCheckResourceAttrPair(resourceName, "instance_class", sourceDbResourceName, "instance_class"), + resource.TestCheckResourceAttrPair(resourceName, "allocated_storage", sourceDbResourceName, "allocated_storage"), + resource.TestCheckResourceAttrPair(resourceName, "engine", sourceDbResourceName, "engine"), + resource.TestCheckResourceAttrPair(resourceName, "engine_version", sourceDbResourceName, "engine_version"), + resource.TestCheckResourceAttrPair(resourceName, "username", sourceDbResourceName, "username"), + resource.TestCheckResourceAttrPair(resourceName, "name", sourceDbResourceName, "name"), + resource.TestCheckResourceAttrPair(resourceName, "maintenance_window", sourceDbResourceName, "maintenance_window"), + resource.TestCheckResourceAttrPair(resourceName, "option_group_name", sourceDbResourceName, "option_group_name"), + resource.TestCheckResourceAttrPair(resourceName, "parameter_group_name", sourceDbResourceName, "parameter_group_name"), + resource.TestCheckResourceAttrPair(resourceName, "port", sourceDbResourceName, "port"), + ), + }, + }, + }) +} + +func TestAccRDSInstance_SnapshotIdentifier_namePrefix(t *testing.T) { + var v rds.DBInstance + + sourceName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + const identifierPrefix = "tf-acc-test-prefix-" + const resourceName = "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_SnapshotIdentifier_namePrefix(identifierPrefix, sourceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + create.TestCheckResourceAttrNameFromPrefix(resourceName, "identifier", identifierPrefix), + resource.TestCheckResourceAttr(resourceName, "identifier_prefix", identifierPrefix), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "password", + "snapshot_identifier", + }, + }, + }, + }) +} + +func TestAccRDSInstance_SnapshotIdentifier_nameGenerated(t *testing.T) { + var v rds.DBInstance + + sourceName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + const resourceName = "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_SnapshotIdentifier_nameGenerated(sourceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + create.TestCheckResourceAttrNameGenerated(resourceName, "identifier"), + resource.TestCheckResourceAttr(resourceName, "identifier_prefix", resource.UniqueIdPrefix), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "password", + "snapshot_identifier", + }, + }, }, }) } -func TestAccRDSInstance_snapshotIdentifierRemoved(t *testing.T) { +func TestAccRDSInstance_SnapshotIdentifier_AssociationRemoved(t *testing.T) { var dbInstance1, dbInstance2 rds.DBInstance rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + sourceDbResourceName := "aws_db_instance.source" resourceName := "aws_db_instance.test" resource.ParallelTest(t, resource.TestCase{ @@ -1567,10 +1647,13 @@ func TestAccRDSInstance_snapshotIdentifierRemoved(t *testing.T) { ), }, { - Config: testAccInstanceConfig_SnapshotIdentifierRemoved(rName), + Config: testAccInstanceConfig_SnapshotIdentifier_AssociationRemoved(rName), Check: resource.ComposeTestCheckFunc( testAccCheckInstanceExists(resourceName, &dbInstance2), testAccCheckInstanceNotRecreated(&dbInstance1, &dbInstance2), + resource.TestCheckResourceAttrPair(resourceName, "allocated_storage", sourceDbResourceName, "allocated_storage"), + resource.TestCheckResourceAttrPair(resourceName, "engine", sourceDbResourceName, "engine"), + resource.TestCheckResourceAttrPair(resourceName, "username", sourceDbResourceName, "username"), ), }, }, @@ -1716,7 +1799,7 @@ func TestAccRDSInstance_SnapshotIdentifier_availabilityZone(t *testing.T) { }) } -func TestAccRDSInstance_SnapshotIdentifier_backupRetentionPeriod(t *testing.T) { +func TestAccRDSInstance_SnapshotIdentifier_backupRetentionPeriod_override(t *testing.T) { var dbInstance, sourceDbInstance rds.DBInstance var dbSnapshot rds.DBSnapshot @@ -1744,7 +1827,7 @@ func TestAccRDSInstance_SnapshotIdentifier_backupRetentionPeriod(t *testing.T) { }) } -func TestAccRDSInstance_SnapshotIdentifierBackupRetentionPeriod_unset(t *testing.T) { +func TestAccRDSInstance_SnapshotIdentifier_backupRetentionPeriod_unset(t *testing.T) { var dbInstance, sourceDbInstance rds.DBInstance var dbSnapshot rds.DBSnapshot @@ -1831,7 +1914,7 @@ func TestAccRDSInstance_SnapshotIdentifier_dbSubnetGroupName(t *testing.T) { }) } -func TestAccRDSInstance_SnapshotIdentifierDBSubnetGroupName_ramShared(t *testing.T) { +func TestAccRDSInstance_SnapshotIdentifier_DBSubnetGroupName_ramShared(t *testing.T) { var dbInstance, sourceDbInstance rds.DBInstance var dbSnapshot rds.DBSnapshot var dbSubnetGroup rds.DBSubnetGroup @@ -1867,7 +1950,7 @@ func TestAccRDSInstance_SnapshotIdentifierDBSubnetGroupName_ramShared(t *testing }) } -func TestAccRDSInstance_SnapshotIdentifierDBSubnetGroupName_vpcSecurityGroupIDs(t *testing.T) { +func TestAccRDSInstance_SnapshotIdentifier_DBSubnetGroupName_vpcSecurityGroupIDs(t *testing.T) { var dbInstance, sourceDbInstance rds.DBInstance var dbSnapshot rds.DBSnapshot var dbSubnetGroup rds.DBSubnetGroup @@ -2076,7 +2159,7 @@ func TestAccRDSInstance_SnapshotIdentifier_multiAZ(t *testing.T) { }) } -func TestAccRDSInstance_SnapshotIdentifierMultiAZ_sqlServer(t *testing.T) { +func TestAccRDSInstance_SnapshotIdentifier_multiAZ_sqlServer(t *testing.T) { var dbInstance, sourceDbInstance rds.DBInstance var dbSnapshot rds.DBSnapshot @@ -2190,7 +2273,7 @@ func TestAccRDSInstance_SnapshotIdentifier_tags(t *testing.T) { }) } -func TestAccRDSInstance_SnapshotIdentifierTags_unset(t *testing.T) { +func TestAccRDSInstance_SnapshotIdentifier_Tags_Clear(t *testing.T) { acctest.Skip(t, "To be fixed: https://github.com/hashicorp/terraform-provider-aws/issues/5959") // --- FAIL: TestAccRDSInstance_SnapshotIdentifierTags_unset (1086.15s) // testing.go:527: Step 0 error: Check failed: Check 4/4 error: aws_db_instance.test: Attribute 'tags.%' expected "0", got "1" @@ -2210,7 +2293,7 @@ func TestAccRDSInstance_SnapshotIdentifierTags_unset(t *testing.T) { CheckDestroy: testAccCheckInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccInstanceConfig_SnapshotIdentifier_Tags_Unset(rName), + Config: testAccInstanceConfig_SnapshotIdentifier_Tags_Clear(rName), Check: resource.ComposeTestCheckFunc( testAccCheckInstanceExists(sourceDbResourceName, &sourceDbInstance), testAccCheckDbSnapshotExists(snapshotResourceName, &dbSnapshot), @@ -2253,7 +2336,7 @@ func TestAccRDSInstance_SnapshotIdentifier_vpcSecurityGroupIDs(t *testing.T) { // This acceptance test explicitly tests when snapshot_identifier is set, // vpc_security_group_ids is set (which triggered the resource update function), // and tags is set which was missing its ARN used for tagging -func TestAccRDSInstance_SnapshotIdentifierVPCSecurityGroupIDs_tags(t *testing.T) { +func TestAccRDSInstance_SnapshotIdentifier_vpcSecurityGroupIDs_tags(t *testing.T) { var dbInstance, sourceDbInstance rds.DBInstance var dbSnapshot rds.DBSnapshot @@ -3848,7 +3931,7 @@ func testAccInstanceConfig_orderableClassMySQL() string { } func testAccInstanceConfig_orderableClassMariadb() string { - return testAccInstanceConfig_orderableClass("mariadb", "10.2.15", "general-public-license") + return testAccInstanceConfig_orderableClass("mariadb", "10.5.12", "general-public-license") } func testAccInstanceConfig_orderableClassSQLServerEx() string { @@ -7061,12 +7144,42 @@ resource "aws_db_instance" "test" { `, rName)) } -func testAccInstanceConfig_SnapshotIdentifierRemoved(rName string) string { - return acctest.ConfigCompose(testAccInstanceConfig_orderableClassMariadb(), fmt.Sprintf(` +func testAccInstanceConfig_SnapshotIdentifier_namePrefix(identifierPrefix, sourceName string) string { + return acctest.ConfigCompose( + testAccInstanceConfig_orderableClassMariadb(), + fmt.Sprintf(` resource "aws_db_instance" "source" { allocated_storage = 5 engine = data.aws_rds_orderable_db_instance.test.engine - identifier = "%[1]s-source" + identifier = %[1]q + instance_class = data.aws_rds_orderable_db_instance.test.instance_class + password = "avoid-plaintext-passwords" + username = "tfacctest" + skip_final_snapshot = true +} + +resource "aws_db_snapshot" "test" { + db_instance_identifier = aws_db_instance.source.id + db_snapshot_identifier = %[1]q +} + +resource "aws_db_instance" "test" { + identifier_prefix = %[2]q + instance_class = aws_db_instance.source.instance_class + snapshot_identifier = aws_db_snapshot.test.id + skip_final_snapshot = true +} +`, sourceName, identifierPrefix)) +} + +func testAccInstanceConfig_SnapshotIdentifier_nameGenerated(sourceName string) string { + return acctest.ConfigCompose( + testAccInstanceConfig_orderableClassMariadb(), + fmt.Sprintf(` +resource "aws_db_instance" "source" { + allocated_storage = 5 + engine = data.aws_rds_orderable_db_instance.test.engine + identifier = %[1]q instance_class = data.aws_rds_orderable_db_instance.test.instance_class password = "avoid-plaintext-passwords" username = "tfacctest" @@ -7079,13 +7192,39 @@ resource "aws_db_snapshot" "test" { } resource "aws_db_instance" "test" { + instance_class = aws_db_instance.source.instance_class + snapshot_identifier = aws_db_snapshot.test.id + skip_final_snapshot = true +} +`, sourceName)) +} + +func testAccInstanceConfig_SnapshotIdentifier_AssociationRemoved(rName string) string { + return acctest.ConfigCompose(testAccInstanceConfig_orderableClassMariadb(), fmt.Sprintf(` +resource "aws_db_instance" "source" { allocated_storage = 5 engine = data.aws_rds_orderable_db_instance.test.engine + identifier = "%[1]s-source" + instance_class = data.aws_rds_orderable_db_instance.test.instance_class password = "avoid-plaintext-passwords" username = "tfacctest" + skip_final_snapshot = true +} + +resource "aws_db_snapshot" "test" { + db_instance_identifier = aws_db_instance.source.id + db_snapshot_identifier = %[1]q +} + +resource "aws_db_instance" "test" { identifier = %[1]q instance_class = aws_db_instance.source.instance_class skip_final_snapshot = true + +// allocated_storage = 5 +// engine = data.aws_rds_orderable_db_instance.test.engine +// password = "avoid-plaintext-passwords" +// username = "tfacctest" } `, rName)) } @@ -7877,6 +8016,10 @@ resource "aws_db_instance" "source" { password = "avoid-plaintext-passwords" username = "tfacctest" skip_final_snapshot = true + + tags = { + key1 = "value-old" + } } resource "aws_db_snapshot" "test" { @@ -7897,8 +8040,10 @@ resource "aws_db_instance" "test" { `, rName)) } -func testAccInstanceConfig_SnapshotIdentifier_Tags_Unset(rName string) string { - return acctest.ConfigCompose(testAccInstanceConfig_orderableClassMariadb(), fmt.Sprintf(` +func testAccInstanceConfig_SnapshotIdentifier_Tags_Clear(rName string) string { + return acctest.ConfigCompose( + testAccInstanceConfig_orderableClassMariadb(), + fmt.Sprintf(` resource "aws_db_instance" "source" { allocated_storage = 5 engine = data.aws_rds_orderable_db_instance.test.engine From cabfee09ab83e4d471645d440dec2b0e418311f6 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 10 Dec 2021 16:27:10 -0800 Subject: [PATCH 09/15] Adds standard naming tests for RDS Instance S3 import restores --- internal/service/rds/enum.go | 1 + internal/service/rds/instance_test.go | 218 ++++++++++++++++++++------ internal/service/rds/wait.go | 1 + 3 files changed, 176 insertions(+), 44 deletions(-) diff --git a/internal/service/rds/enum.go b/internal/service/rds/enum.go index e08b834fe1e..c31b4aa776e 100644 --- a/internal/service/rds/enum.go +++ b/internal/service/rds/enum.go @@ -29,6 +29,7 @@ const ( InstanceStatusCreating = "creating" InstanceStatusDeleting = "deleting" InstanceStatusIncompatibleParameters = "incompatible-parameters" + InstanceStatusIncompatibleRestore = "incompatible-restore" InstanceStatusModifying = "modifying" InstanceStatusStarting = "starting" InstanceStatusStopping = "stopping" diff --git a/internal/service/rds/instance_test.go b/internal/service/rds/instance_test.go index ee1d4b379e4..173c61f9e83 100644 --- a/internal/service/rds/instance_test.go +++ b/internal/service/rds/instance_test.go @@ -1499,12 +1499,14 @@ func TestAccRDSInstance_ReplicateSourceDB_parameterGroupTwoStep(t *testing.T) { }) } -func TestAccRDSInstance_s3Import(t *testing.T) { +func TestAccRDSInstance_s3Import_basic(t *testing.T) { var snap rds.DBInstance bucket := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) uniqueId := sdkacctest.RandomWithPrefix("tf-acc-s3-import-test") bucketPrefix := sdkacctest.RandString(5) + const resourceName = "aws_db_instance.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), @@ -1512,11 +1514,88 @@ func TestAccRDSInstance_s3Import(t *testing.T) { CheckDestroy: testAccCheckInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccInstanceConfig_S3Import(bucket, bucketPrefix, uniqueId), + Config: testAccInstanceConfig_S3Import_basic(bucket, bucketPrefix, uniqueId), Check: resource.ComposeTestCheckFunc( - testAccCheckInstanceExists("aws_db_instance.s3", &snap), + testAccCheckInstanceExists(resourceName, &snap), + resource.TestCheckResourceAttr(resourceName, "identifier", uniqueId), + resource.TestCheckResourceAttr(resourceName, "identifier_prefix", ""), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "password", + }, + }, + }, + }) +} + +func TestAccRDSInstance_s3Import_namePrefix(t *testing.T) { + var snap rds.DBInstance + const identifierPrefix = "tf-acc-test-prefix-" + bucket := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + bucketPrefix := sdkacctest.RandString(5) + + const resourceName = "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_S3Import_namePrefix(bucket, bucketPrefix, identifierPrefix), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &snap), + create.TestCheckResourceAttrNameFromPrefix(resourceName, "identifier", identifierPrefix), + resource.TestCheckResourceAttr(resourceName, "identifier_prefix", identifierPrefix), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "password", + }, + }, + }, + }) +} + +func TestAccRDSInstance_s3Import_nameGenerated(t *testing.T) { + var snap rds.DBInstance + bucket := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + bucketPrefix := sdkacctest.RandString(5) + + const resourceName = "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_S3Import_nameGenerated(bucket, bucketPrefix), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &snap), + create.TestCheckResourceAttrNameGenerated(resourceName, "identifier"), + resource.TestCheckResourceAttr(resourceName, "identifier_prefix", resource.UniqueIdPrefix), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "password", + }, + }, }, }) } @@ -4182,8 +4261,10 @@ resource "aws_db_instance" "snapshot" { `, sdkacctest.RandInt())) } -func testAccInstanceConfig_S3Import(bucketName string, bucketPrefix string, uniqueId string) string { - return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(` +func testAccInstanceConfig_S3Import_Base(rName, bucketPrefix string) string { + return acctest.ConfigCompose( + acctest.ConfigVpcWithSubnets(2), + fmt.Sprintf(` resource "aws_s3_bucket" "xtrabackup" { bucket = %[1]q } @@ -4198,7 +4279,7 @@ resource "aws_s3_object" "xtrabackup_db" { data "aws_partition" "current" {} resource "aws_iam_role" "rds_s3_access_role" { - name = "%[3]s-role" + name = "%[1]s-role" assume_role_policy = < Date: Fri, 10 Dec 2021 17:32:17 -0800 Subject: [PATCH 10/15] Consistently set identifier on creation --- internal/service/rds/instance.go | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/internal/service/rds/instance.go b/internal/service/rds/instance.go index 5d7e1b77636..7255b26dafb 100644 --- a/internal/service/rds/instance.go +++ b/internal/service/rds/instance.go @@ -518,18 +518,7 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { // we expect everything to be in sync before returning completion. var requiresRebootDbInstance bool - var identifier string - if v, ok := d.GetOk("identifier"); ok { - identifier = v.(string) - } else { - if v, ok := d.GetOk("identifier_prefix"); ok { - identifier = resource.PrefixedUniqueId(v.(string)) - } else { - identifier = resource.UniqueId() - } - - d.Set("identifier", identifier) - } + identifier := create.Name(d.Get("identifier").(string), d.Get("identifier_prefix").(string)) if v, ok := d.GetOk("replicate_source_db"); ok { opts := rds.CreateDBInstanceReadReplicaInput{ @@ -725,7 +714,7 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { CopyTagsToSnapshot: aws.Bool(d.Get("copy_tags_to_snapshot").(bool)), DBName: aws.String(d.Get("name").(string)), DBInstanceClass: aws.String(d.Get("instance_class").(string)), - DBInstanceIdentifier: aws.String(d.Get("identifier").(string)), + DBInstanceIdentifier: aws.String(identifier), DeletionProtection: aws.Bool(d.Get("deletion_protection").(bool)), Engine: aws.String(d.Get("engine").(string)), EngineVersion: aws.String(d.Get("engine_version").(string)), @@ -859,7 +848,7 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("Error creating DB Instance: %s", err) } - d.SetId(d.Get("identifier").(string)) + d.SetId(identifier) log.Printf("[INFO] DB Instance ID: %s", d.Id()) @@ -887,7 +876,7 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { AutoMinorVersionUpgrade: aws.Bool(d.Get("auto_minor_version_upgrade").(bool)), CopyTagsToSnapshot: aws.Bool(d.Get("copy_tags_to_snapshot").(bool)), DBInstanceClass: aws.String(d.Get("instance_class").(string)), - DBInstanceIdentifier: aws.String(d.Get("identifier").(string)), + DBInstanceIdentifier: aws.String(identifier), DBSnapshotIdentifier: aws.String(d.Get("snapshot_identifier").(string)), DeletionProtection: aws.Bool(d.Get("deletion_protection").(bool)), PubliclyAccessible: aws.Bool(d.Get("publicly_accessible").(bool)), @@ -1084,7 +1073,7 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { input.DeletionProtection = aws.Bool(d.Get("deletion_protection").(bool)) input.PubliclyAccessible = aws.Bool(d.Get("publicly_accessible").(bool)) input.Tags = Tags(tags.IgnoreAWS()) - input.TargetDBInstanceIdentifier = aws.String(d.Get("identifier").(string)) + input.TargetDBInstanceIdentifier = aws.String(identifier) if v, ok := d.GetOk("availability_zone"); ok { input.AvailabilityZone = aws.String(v.(string)) @@ -1187,7 +1176,7 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { AllocatedStorage: aws.Int64(int64(d.Get("allocated_storage").(int))), DBName: aws.String(d.Get("name").(string)), DBInstanceClass: aws.String(d.Get("instance_class").(string)), - DBInstanceIdentifier: aws.String(d.Get("identifier").(string)), + DBInstanceIdentifier: aws.String(identifier), DeletionProtection: aws.Bool(d.Get("deletion_protection").(bool)), MasterUsername: aws.String(d.Get("username").(string)), MasterUserPassword: aws.String(d.Get("password").(string)), @@ -1345,7 +1334,7 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { } } - d.SetId(d.Get("identifier").(string)) + d.SetId(identifier) stateConf := &resource.StateChangeConf{ Pending: resourceInstanceCreatePendingStates, From 48b79463a72781858b69f2a664a9322617174c29 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 10 Dec 2021 17:32:29 -0800 Subject: [PATCH 11/15] Cleanup --- internal/service/rds/instance.go | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/internal/service/rds/instance.go b/internal/service/rds/instance.go index 7255b26dafb..54e6adcff03 100644 --- a/internal/service/rds/instance.go +++ b/internal/service/rds/instance.go @@ -845,15 +845,14 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { _, err = conn.RestoreDBInstanceFromS3(&opts) } if err != nil { - return fmt.Errorf("Error creating DB Instance: %s", err) + return fmt.Errorf("Error creating DB Instance: %w", err) } d.SetId(identifier) log.Printf("[INFO] DB Instance ID: %s", d.Id()) - log.Println( - "[INFO] Waiting for DB Instance to be available") + log.Println("[INFO] Waiting for DB Instance to be available") stateConf := &resource.StateChangeConf{ Pending: resourceInstanceCreatePendingStates, @@ -1063,7 +1062,7 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { } if err != nil { - return fmt.Errorf("Error creating DB Instance: %s", err) + return fmt.Errorf("Error creating DB Instance: %w", err) } } else if v, ok := d.GetOk("restore_to_point_in_time"); ok { if input := expandRestoreToPointInTime(v.([]interface{})); input != nil { @@ -1323,9 +1322,9 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { if err != nil { if tfawserr.ErrMessageContains(err, "InvalidParameterValue", "") { opts.MasterUserPassword = aws.String("********") - return fmt.Errorf("Error creating DB Instance: %s, %+v", err, opts) + return fmt.Errorf("Error creating DB Instance: %w, %+v", err, opts) } - return fmt.Errorf("Error creating DB Instance: %s", err) + return fmt.Errorf("Error creating DB Instance: %w", err) } // This is added here to avoid unnecessary modification when ca_cert_identifier is the default one if attr, ok := d.GetOk("ca_cert_identifier"); ok && attr.(string) != aws.StringValue(createdDBInstanceOutput.DBInstance.CACertificateIdentifier) { @@ -1357,13 +1356,13 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { log.Printf("[INFO] DB Instance (%s) configuration requires ModifyDBInstance: %s", d.Id(), modifyDbInstanceInput) _, err := conn.ModifyDBInstance(modifyDbInstanceInput) if err != nil { - return fmt.Errorf("error modifying DB Instance (%s): %s", d.Id(), err) + return fmt.Errorf("error modifying DB Instance (%s): %w", d.Id(), err) } log.Printf("[INFO] Waiting for DB Instance (%s) to be available", d.Id()) err = waitUntilDBInstanceAvailableAfterUpdate(d.Id(), conn, d.Timeout(schema.TimeoutUpdate)) if err != nil { - return fmt.Errorf("error waiting for DB Instance (%s) to be available: %s", d.Id(), err) + return fmt.Errorf("error waiting for DB Instance (%s) to be available: %w", d.Id(), err) } } @@ -1375,13 +1374,13 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error { log.Printf("[INFO] DB Instance (%s) configuration requires RebootDBInstance: %s", d.Id(), rebootDbInstanceInput) _, err := conn.RebootDBInstance(rebootDbInstanceInput) if err != nil { - return fmt.Errorf("error rebooting DB Instance (%s): %s", d.Id(), err) + return fmt.Errorf("error rebooting DB Instance (%s): %w", d.Id(), err) } log.Printf("[INFO] Waiting for DB Instance (%s) to be available", d.Id()) err = waitUntilDBInstanceAvailableAfterUpdate(d.Id(), conn, d.Timeout(schema.TimeoutUpdate)) if err != nil { - return fmt.Errorf("error waiting for DB Instance (%s) to be available: %s", d.Id(), err) + return fmt.Errorf("error waiting for DB Instance (%s) to be available: %w", d.Id(), err) } } @@ -1466,7 +1465,7 @@ func resourceInstanceRead(d *schema.ResourceData, meta interface{}) error { d.Set("monitoring_role_arn", v.MonitoringRoleArn) if err := d.Set("enabled_cloudwatch_logs_exports", flex.FlattenStringList(v.EnabledCloudwatchLogsExports)); err != nil { - return fmt.Errorf("error setting enabled_cloudwatch_logs_exports: %s", err) + return fmt.Errorf("error setting enabled_cloudwatch_logs_exports: %w", err) } d.Set("domain", "") @@ -1482,7 +1481,7 @@ func resourceInstanceRead(d *schema.ResourceData, meta interface{}) error { tags, err := ListTags(conn, d.Get("arn").(string)) if err != nil { - return fmt.Errorf("error listing tags for RDS DB Instance (%s): %s", d.Get("arn").(string), err) + return fmt.Errorf("error listing tags for RDS DB Instance (%s): %w", d.Get("arn").(string), err) } tags = tags.IgnoreAWS().IgnoreConfig(ignoreTagsConfig) @@ -1791,13 +1790,13 @@ func resourceInstanceUpdate(d *schema.ResourceData, meta interface{}) error { } if err != nil { - return fmt.Errorf("Error modifying DB Instance %s: %s", d.Id(), err) + return fmt.Errorf("Error modifying DB Instance %s: %w", d.Id(), err) } log.Printf("[DEBUG] Waiting for DB Instance (%s) to be available", d.Id()) err = waitUntilDBInstanceAvailableAfterUpdate(d.Id(), conn, d.Timeout(schema.TimeoutUpdate)) if err != nil { - return fmt.Errorf("error waiting for DB Instance (%s) to be available: %s", d.Id(), err) + return fmt.Errorf("error waiting for DB Instance (%s) to be available: %w", d.Id(), err) } } @@ -1827,7 +1826,7 @@ func resourceInstanceUpdate(d *schema.ResourceData, meta interface{}) error { o, n := d.GetChange("tags_all") if err := UpdateTags(conn, d.Get("arn").(string), o, n); err != nil { - return fmt.Errorf("error updating RDS DB Instance (%s) tags: %s", d.Get("arn").(string), err) + return fmt.Errorf("error updating RDS DB Instance (%s) tags: %w", d.Get("arn").(string), err) } } @@ -1851,7 +1850,7 @@ func resourceInstanceRetrieve(id string, conn *rds.RDS) (*rds.DBInstance, error) if tfawserr.ErrMessageContains(err, rds.ErrCodeDBInstanceNotFoundFault, "") { return nil, nil } - return nil, fmt.Errorf("Error retrieving DB Instances: %s", err) + return nil, fmt.Errorf("Error retrieving DB Instances: %w", err) } if len(resp.DBInstances) != 1 || resp.DBInstances[0] == nil || aws.StringValue(resp.DBInstances[0].DBInstanceIdentifier) != id { From 8526ae75a77e27dff3813048480c6411f3c033a0 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 10 Dec 2021 17:41:19 -0800 Subject: [PATCH 12/15] Adds CHANGELOG entry --- .changelog/22178.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/22178.txt diff --git a/.changelog/22178.txt b/.changelog/22178.txt new file mode 100644 index 00000000000..4f90a78c96c --- /dev/null +++ b/.changelog/22178.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_db_instance: Fix error with reboot of replica +``` From 748a8ffaadd1a52f15939d98c0d034882cea9755 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 10 Dec 2021 17:56:28 -0800 Subject: [PATCH 13/15] Clean up --- internal/service/rds/instance_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/internal/service/rds/instance_test.go b/internal/service/rds/instance_test.go index 173c61f9e83..a6a7129dc15 100644 --- a/internal/service/rds/instance_test.go +++ b/internal/service/rds/instance_test.go @@ -7350,11 +7350,6 @@ resource "aws_db_instance" "test" { identifier = %[1]q instance_class = aws_db_instance.source.instance_class skip_final_snapshot = true - -// allocated_storage = 5 -// engine = data.aws_rds_orderable_db_instance.test.engine -// password = "avoid-plaintext-passwords" -// username = "tfacctest" } `, rName)) } From 9c2c9509d991d64fe1d66985cbfba98af54ab203 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 2 Feb 2022 15:28:23 -0800 Subject: [PATCH 14/15] Update MariaDB version --- internal/service/rds/instance_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/rds/instance_test.go b/internal/service/rds/instance_test.go index a6a7129dc15..63f0d87c889 100644 --- a/internal/service/rds/instance_test.go +++ b/internal/service/rds/instance_test.go @@ -7385,7 +7385,7 @@ func testAccInstanceConfig_SnapshotIdentifier_Io1Storage(rName string, iops int) return fmt.Sprintf(` data "aws_rds_orderable_db_instance" "test" { engine = "mariadb" - engine_version = "10.2.15" + engine_version = "10.5.12" license_model = "general-public-license" storage_type = "io1" From 2f03e81aca5561e38e89e737745222006a7904d5 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 2 Feb 2022 16:03:48 -0800 Subject: [PATCH 15/15] Fix package name in service filters generator --- .../namevaluesfilters/generators/servicefilters/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/generate/namevaluesfilters/generators/servicefilters/main.go b/internal/generate/namevaluesfilters/generators/servicefilters/main.go index fd4ffb3d612..d6d18ce42c1 100644 --- a/internal/generate/namevaluesfilters/generators/servicefilters/main.go +++ b/internal/generate/namevaluesfilters/generators/servicefilters/main.go @@ -12,7 +12,7 @@ import ( "strings" "text/template" - "github.com/hashicorp/terraform-provider-aws/internal/namevaluesfilters" + "github.com/hashicorp/terraform-provider-aws/internal/generate/namevaluesfilters" ) const filename = `service_filters_gen.go`